SteNicholas opened a new pull request, #464:
URL: https://github.com/apache/paimon-cpp/pull/464
<!-- PR titles must follow Conventional Commits: <type>(<optional-scope>):
<description> -->
### Purpose
<!-- Linking this pull request to the issue -->
Linked issue: close #463
<!-- What is the purpose of the change -->
`Logger::GetLogger` read the registered creator before taking the registry
lock,
which raced with the first `Logger::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 `std::shared_ptr`, copy the pointer under
the
shared 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
<!-- List UT and IT cases to verify this change -->
Unit tests in `src/paimon/common/logging/logging_test.cpp`. Each runs in an
isolated
child process because a registered creator cannot be unset:
- `TestRegisterStatefulLoggerCreator`: verifies that a creator with internal
state
keeps it across `GetLogger` calls.
- `TestRegisterLoggerCreatorWhileGettingLogger`: registers the first creator
while
another thread gets loggers. ThreadSanitizer reports a data race if
`GetLogger`
reads the creator without holding the lock.
- `TestRegisterLoggerFromLoggerCreator`: registers a creator from inside the
running
creator, and verifies that `GetLogger` does not deadlock and that the
replaced
creator's state stays alive until the call finishes.
### API and Format
<!-- Does this change affect API in include dir or storage format or
protocol -->
No API signature, storage format, or protocol changes.
`include/paimon/logging.h`
only adds documentation for `RegisterLogger` and `GetLogger`.
### Documentation
<!-- Does this change introduce a new feature -->
The public header now documents the thread safety of `RegisterLogger` and
`GetLogger` and what they require of the creator. No other documentation
changes
are required for this bug fix.
### Generative AI tooling
<!--
If generative AI tooling has been used in the process of authoring this
patch, please include the
phrase: 'Generated-by: ' followed by the name of the tool and its version.
If no, write 'No'.
Please refer to the [ASF Generative Tooling
Guidance](https://www.apache.org/legal/generative-tooling.html) for details.
-->
Generated-by: Claude Code 2.1.310 (Claude Opus 5.5)
🤖 Generated with [Claude Code](https://claude.com/claude-code)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
