vanand123 commented on issue #545: URL: https://github.com/apache/logging-log4cxx/issues/545#issuecomment-3489752343
@rm5248 I'm not able to create a standalone program to reproduce this issue, but I have tried a minimal fix as below which is not leading to the previously mentioned crash. ## Root Cause Analysis The crash occurs due to a race condition in `Logger::getLoggerRepository()` where it returns a raw pointer (`m_priv->repositoryRaw`) that can become dangling when the repository is destroyed/recreated by another thread during multithreaded logger initialization. **Crash sequence:** 1. Thread A calls `logger->getLoggerRepository()` → gets raw pointer 2. Thread B triggers repository lifecycle changes 3. Repository gets destroyed while Thread A still holds the raw pointer 4. Thread A uses stale pointer → **SEGFAULT** in `LogManager::getLoggerLS()` ## Proposed Minimal Fix After analyzing [PR #94](https://github.com/apache/logging-log4cxx/pull/94), I found a simpler approach that achieves the same safety goals without the complexity of weak_ptr management: ```diff diff --git a/src/main/cpp/logger.cpp b/src/main/cpp/logger.cpp index 50fbf37..ultra_minimal 100644 --- a/src/main/cpp/logger.cpp +++ b/src/main/cpp/logger.cpp @@ -289,9 +289,10 @@ const LevelPtr Logger::getEffectiveLevel() const #endif } -LoggerRepository* Logger::getLoggerRepository() const +spi::LoggerRepositoryPtr Logger::getLoggerRepository() const { - return m_priv->repositoryRaw; + // Fix LOGCXX-546: Return shared_ptr to prevent use-after-free + return LogManager::getLoggerRepository(); } LoggerRepository* Logger::getHierarchy() const diff --git a/src/main/include/log4cxx/logger.h b/src/main/include/log4cxx/logger.h index c3d991d..ultra_minimal 100644 --- a/src/main/include/log4cxx/logger.h +++ b/src/main/include/log4cxx/logger.h @@ -788,7 +788,7 @@ class LOG4CXX_EXPORT Logger Return the the LoggerRepository where this <code>Logger</code> is attached. */ - spi::LoggerRepository* getLoggerRepository() const; + spi::LoggerRepositoryPtr getLoggerRepository() const; /** * Get the logger name. ``` ## Why This Fix Works **Key insight**: Instead of managing complex weak_ptr lifecycles, we delegate to `LogManager::getLoggerRepository()` which: - Returns a proper `shared_ptr` (not raw pointer) - Provides the current, authoritative repository - Prevents use-after-free through reference counting - Is thread-safe by design **Testing results:** - No crashes in multithreaded stress testing - All existing functionality preserved - Memory safety verified Is this approach fine to fix the issue? Let me know if there is some other suggestion from your side. -- 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]
