Skip to content

Commit

Permalink
Fix data race in ExprToSubfieldFilterParser::getInstance (#11513)
Browse files Browse the repository at this point in the history
Summary:
Pull Request resolved: #11513

Racing condition found with TSAN when accessing the singleton parser factory.

Reviewed By: xiaoxmeng

Differential Revision: D65824456

fbshipit-source-id: 07f2af863ac8bfc00028650103ad492f05c5ed64
  • Loading branch information
Yuhta authored and facebook-github-bot committed Nov 12, 2024
1 parent 551fc2f commit 7437093
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 24 deletions.
25 changes: 5 additions & 20 deletions velox/expression/ExprToSubfieldFilter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -112,28 +112,13 @@ toInt64List(const VectorPtr& vector, vector_size_t start, vector_size_t size) {
return values;
}

} // namespace

std::function<std::unique_ptr<ExprToSubfieldFilterParser>()>
ExprToSubfieldFilterParser::parserFactory_ = nullptr;
static std::shared_ptr<ExprToSubfieldFilterParser> defaultParser =
std::make_shared<PrestoExprToSubfieldFilterParser>();

// static
std::unique_ptr<ExprToSubfieldFilterParser>
ExprToSubfieldFilterParser::getInstance() {
if (!parserFactory_) {
parserFactory_ = []() {
return std::make_unique<PrestoExprToSubfieldFilterParser>();
};
}
return parserFactory_();
}
} // namespace

// static
void ExprToSubfieldFilterParser::registerParserFactory(
std::function<std::unique_ptr<ExprToSubfieldFilterParser>()>
parserFactory) {
parserFactory_ = parserFactory;
}
std::function<std::shared_ptr<ExprToSubfieldFilterParser>()>
ExprToSubfieldFilterParser::parserFactory_ = [] { return defaultParser; };

bool ExprToSubfieldFilterParser::toSubfield(
const core::ITypedExpr* field,
Expand Down
12 changes: 8 additions & 4 deletions velox/expression/ExprToSubfieldFilter.h
Original file line number Diff line number Diff line change
Expand Up @@ -418,13 +418,17 @@ class ExprToSubfieldFilterParser {
public:
virtual ~ExprToSubfieldFilterParser() = default;

static std::unique_ptr<ExprToSubfieldFilterParser> getInstance();
static std::shared_ptr<ExprToSubfieldFilterParser> getInstance() {
return parserFactory_();
}

/// Registers a custom parser factory. The factory is called to create a
/// parser instance.
static void registerParserFactory(
std::function<std::unique_ptr<ExprToSubfieldFilterParser>()>
parserFactory);
std::function<std::shared_ptr<ExprToSubfieldFilterParser>()>
parserFactory) {
parserFactory_ = std::move(parserFactory);
}

/// Converts a leaf call expression (no conjunction like AND/OR) to subfield
/// and filter. Return nullptr if not supported for pushdown. This is needed
Expand Down Expand Up @@ -488,7 +492,7 @@ class ExprToSubfieldFilterParser {

private:
// Factory method to create a parser instance.
static std::function<std::unique_ptr<ExprToSubfieldFilterParser>()>
static std::function<std::shared_ptr<ExprToSubfieldFilterParser>()>
parserFactory_;
};

Expand Down

0 comments on commit 7437093

Please sign in to comment.