From f54787b7319ff0116172a3dcabb0019080aceb80 Mon Sep 17 00:00:00 2001 From: Bikramjeet Vig Date: Fri, 3 May 2024 13:57:18 -0700 Subject: [PATCH] Fix data race when resetting lazy vector's consistency check (#9701) Summary: Pull Request resolved: https://github.com/facebookincubator/velox/pull/9701 Lazy vector's consistency check is implemented using a member variable `containsLazyAndIsWrapped_`. A recent change ensured that it is reset whenever the encoding layer wrapping it is destroyed which allows it to be wrapped again with another layer. This however showed up as a data race under TSAN because a non-lazy vector can be wrapped by multiple encoding layers at the same level and can therefore be potentially cleared concurrently when they are destroyed. This change fixes that data race. Reviewed By: mbasmanova Differential Revision: D56942275 fbshipit-source-id: f42221ce92e3e822de909c486d250141cac9106e --- velox/vector/BaseVector.h | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/velox/vector/BaseVector.h b/velox/vector/BaseVector.h index d2d6627325c8..7a6b2d9349e2 100644 --- a/velox/vector/BaseVector.h +++ b/velox/vector/BaseVector.h @@ -795,7 +795,9 @@ class BaseVector { } void clearContainingLazyAndWrapped() { - containsLazyAndIsWrapped_ = false; + if (containsLazyAndIsWrapped_) { + containsLazyAndIsWrapped_ = false; + } } bool memoDisabled() const { @@ -919,7 +921,7 @@ class BaseVector { // unloaded lazy vector should not be wrapped by two separate top level // vectors. This would ensure we avoid it being loaded for two separate set // of rows. - bool containsLazyAndIsWrapped_{false}; + std::atomic_bool containsLazyAndIsWrapped_{false}; // Whether we should use Expr::evalWithMemo to cache the result of evaluation // on dictionary values (this vector). Set to false when the dictionary