Repository navigation
fix(logging): read logger creator under lock in GetLogger - #464
Merged
Merged
Conversation
GetLogger read the registered creator before taking the registry lock, which raced with the first RegisterLogger call. It also called the creator while holding the shared lock, so a creator that calls RegisterLogger could deadlock. Without a registered creator, every call took the exclusive lock just to check whether glog was initialized. Hold the registered creator in a shared_ptr, copy the pointer under the shared lock, and call the creator after unlocking, so it keeps its state across calls and stays alive if replaced. Initialize glog once with std::call_once. Document the thread safety of RegisterLogger and GetLogger, and what they require of the creator.
lucasfang
pushed a commit
to lucasfang/paimon-cpp
that referenced
this pull request
Oct 27, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Purpose
Linked issue: close #463
Logger::GetLoggerread the registered creator before taking the registry lock,which raced with the first
Logger::RegisterLoggercall. It also called thecreator while holding the shared lock, so a creator that calls
RegisterLoggercould deadlock. Without a registered creator, every call took the exclusive lock
just to check whether glog was initialized.
Hold the registered creator in a
std::shared_ptr, copy the pointer under theshared lock, and call the creator after unlocking. All calls share the registered
creator, so its state persists across calls, and a creator replaced while it runs
stays alive until that call finishes. Initialize glog once with
std::call_once.Tests
Unit tests in
src/paimon/common/logging/logging_test.cpp. Each runs in an isolatedchild process because a registered creator cannot be unset:
TestRegisterStatefulLoggerCreator: verifies that a creator with internal statekeeps it across
GetLoggercalls.TestRegisterLoggerCreatorWhileGettingLogger: registers the first creator whileanother thread gets loggers. ThreadSanitizer reports a data race if
GetLoggerreads the creator without holding the lock.
TestRegisterLoggerFromLoggerCreator: registers a creator from inside the runningcreator, and verifies that
GetLoggerdoes not deadlock and that the replacedcreator's state stays alive until the call finishes.
API and Format
No API signature, storage format, or protocol changes.
include/paimon/logging.honly adds documentation for
RegisterLoggerandGetLogger.Documentation
The public header now documents the thread safety of
RegisterLoggerandGetLoggerand what they require of the creator. No other documentation changesare required for this bug fix.
Generative AI tooling
Generated-by: Claude Code 2.1.310 (Claude Opus 5.5)
🤖 Generated with Claude Code