Skip to content

Commit

Permalink
Fix data race when resetting lazy vector's consistency check (faceboo…
Browse files Browse the repository at this point in the history
…kincubator#9701)

Summary:
Pull Request resolved: facebookincubator#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
  • Loading branch information
Bikramjeet Vig authored and facebook-github-bot committed May 3, 2024
1 parent 5bc6818 commit f54787b
Showing 1 changed file with 4 additions and 2 deletions.
6 changes: 4 additions & 2 deletions velox/vector/BaseVector.h
Original file line number Diff line number Diff line change
Expand Up @@ -795,7 +795,9 @@ class BaseVector {
}

void clearContainingLazyAndWrapped() {
containsLazyAndIsWrapped_ = false;
if (containsLazyAndIsWrapped_) {
containsLazyAndIsWrapped_ = false;
}
}

bool memoDisabled() const {
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit f54787b

Please sign in to comment.