From dceab4c9a73514cd1de7c8c43e873b2d0f828ab8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=81ukasz=20Osipiuk?= Date: Thu, 9 Jan 2025 17:01:04 +0100 Subject: [PATCH] Ensure visibility of finalSinkMetrics Without exchangeSink not being volatile it was possible that other thread could observe nulled exchangeSink but still not set finalSinkMetrics. --- .../trino/execution/buffer/SpoolingExchangeOutputBuffer.java | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/core/trino-main/src/main/java/io/trino/execution/buffer/SpoolingExchangeOutputBuffer.java b/core/trino-main/src/main/java/io/trino/execution/buffer/SpoolingExchangeOutputBuffer.java index ed615551fd2f..eccf49e01ca9 100644 --- a/core/trino-main/src/main/java/io/trino/execution/buffer/SpoolingExchangeOutputBuffer.java +++ b/core/trino-main/src/main/java/io/trino/execution/buffer/SpoolingExchangeOutputBuffer.java @@ -45,9 +45,7 @@ public class SpoolingExchangeOutputBuffer private volatile SpoolingOutputBuffers outputBuffers; // This field is not final to allow releasing the memory retained by the ExchangeSink instance. // It is modified (assigned to null) when the OutputBuffer is destroyed (either finished or aborted). - // It doesn't have to be declared as volatile as the nullification of this variable doesn't have to be immediately visible to other threads. - // However since the abort can be triggered at any moment of time this variable has to be accessed in a safe way (avoiding "check-then-use"). - private ExchangeSink exchangeSink; + private volatile ExchangeSink exchangeSink; private Optional finalSinkMetrics; private final Supplier memoryContextSupplier;