refactor: make thread_system a required dependency#217
Merged
Conversation
- Remove LOGGER_STANDALONE_MODE option and related conditionals - Change thread_system from OPTIONAL to REQUIRED dependency - Update LoggerSystemConfig.cmake.in to require ThreadSystem - Remove standalone mode conditions from tests and examples CMake - Clean up UnifiedDependencies.cmake standalone mode settings Part of #216
- Update thread_integration_detector.h to always define USE_THREAD_SYSTEM - Remove LOGGER_STANDALONE_MODE checks from monitoring_integration_detector.h - Remove fallback implementation from thread_system_monitor_adapter.h - Simplify di_container_factory.h and monitoring_factory.h - Update log_filter.h to always use thread's logger_interface Part of #216
- Update log_collector.cpp to use log_collector_worker class - Update network_writer.cpp to use network_send_worker and network_reconnect_worker - Update batch_processor.cpp to use batch_processing_worker class - All worker classes extend thread_system's thread_base for consistent thread management - Remove direct std::thread usage in favor of thread_base lifecycle management Part of #216
- Update thread_safety_tests.cpp to always include async_writer - Update thread_system_integration_test.cpp to remove USE_THREAD_SYSTEM conditions - All tests now assume thread_system is a required dependency Part of #216
- Remove --standalone and --with-thread command line options - Remove STANDALONE_MODE and WITH_THREAD variables - thread_system is now a required dependency Part of #216
…ime management Replace thread_base with std::thread and shared_ptr to state in log_collector to prevent use-after-free bugs. The shared state pattern ensures worker callback can safely access data even after impl destruction. - Add log_collector_shared_state struct with shared_ptr-managed lifetime - Add worker reset in network_writer destructor for thread safety - Mark IMonitorable integration as Phase 2.2 TODO in examples - Update CHANGELOG with thread_system required dependency details
edeb770 to
3a0038b
Compare
kcenon
added a commit
that referenced
this pull request
Dec 22, 2025
…0.0 compatibility The file/line/function overload of log() was removed from common::interfaces::ILogger in common_system v3.0.0 (Issue #217). This method is preserved for backward compatibility but no longer overrides the interface method. This fixes the build error: 'log(...)' marked 'override', but does not override
kcenon
added a commit
that referenced
this pull request
Dec 22, 2025
The file/line/function overload of log() was removed from ILogger interface in common_system v3.0.0 (Issue #217). Update test to call the backward-compatible method directly on logger class instead of through ILogger pointer.
kcenon
added a commit
that referenced
this pull request
Dec 22, 2025
) * feat: use KCENON feature detection for jthread and source_location - Replace LOGGER_HAS_SOURCE_LOCATION with KCENON_HAS_SOURCE_LOCATION - Replace LOGGER_HAS_JTHREAD with KCENON_HAS_JTHREAD - Include kcenon/common/config/feature_flags.h for unified detection - Keep LOGGER_HAS_* as legacy aliases for backward compatibility Closes #250 * docs: update changelog for KCENON feature detection migration * fix: remove override from deprecated log method for common_system v3.0.0 compatibility The file/line/function overload of log() was removed from common::interfaces::ILogger in common_system v3.0.0 (Issue #217). This method is preserved for backward compatibility but no longer overrides the interface method. This fixes the build error: 'log(...)' marked 'override', but does not override * test: update ilogger_interface_test for common_system v3.0.0 API changes The file/line/function overload of log() was removed from ILogger interface in common_system v3.0.0 (Issue #217). Update test to call the backward-compatible method directly on logger class instead of through ILogger pointer. * docs: update CHANGELOG for common_system v3.0.0 compatibility fix Document the fix for the deprecated log method override issue and test file update. * ci: use thread_system feature branch for KCENON_HAS_COMMON_EXECUTOR fix Update CI workflows to use thread_system feature branch that includes the fix for KCENON_HAS_COMMON_EXECUTOR definition in submodule builds. This resolves the Windows MSVC LNK2019 unresolved external symbol error for thread_pool::is_running() that was caused by inconsistent class layout between logger_system and thread_system builds. Temporary change until thread_system PR is merged to main. * ci: use UNIFIED_USE_LOCAL for local checkout dependencies Enable UNIFIED_USE_LOCAL=ON in CI workflows to use locally checked out common_system and thread_system instead of FetchContent downloading from main branch. This ensures that the thread_system feature branch with KCENON_HAS_COMMON_EXECUTOR fix is used in the build. * docs: update changelog with Windows MSVC linker fix Document the fix for Windows MSVC LNK2019 unresolved external symbol error for thread_pool::is_running(). Root cause: KCENON_HAS_COMMON_EXECUTOR was not defined when thread_system was built as submodule, causing class layout mismatch. Solution: - thread_system core/CMakeLists.txt defines KCENON_HAS_COMMON_EXECUTOR=1 - CI workflows use UNIFIED_USE_LOCAL=ON for local dependencies
kcenon
added a commit
that referenced
this pull request
Apr 13, 2026
* refactor(cmake): make thread_system a required dependency - Remove LOGGER_STANDALONE_MODE option and related conditionals - Change thread_system from OPTIONAL to REQUIRED dependency - Update LoggerSystemConfig.cmake.in to require ThreadSystem - Remove standalone mode conditions from tests and examples CMake - Clean up UnifiedDependencies.cmake standalone mode settings Part of #216 * refactor(headers): remove conditional compilation for standalone mode - Update thread_integration_detector.h to always define USE_THREAD_SYSTEM - Remove LOGGER_STANDALONE_MODE checks from monitoring_integration_detector.h - Remove fallback implementation from thread_system_monitor_adapter.h - Simplify di_container_factory.h and monitoring_factory.h - Update log_filter.h to always use thread's logger_interface Part of #216 * refactor(threading): migrate std::thread to thread_system's thread_base - Update log_collector.cpp to use log_collector_worker class - Update network_writer.cpp to use network_send_worker and network_reconnect_worker - Update batch_processor.cpp to use batch_processing_worker class - All worker classes extend thread_system's thread_base for consistent thread management - Remove direct std::thread usage in favor of thread_base lifecycle management Part of #216 * refactor(tests): remove standalone mode conditions from tests - Update thread_safety_tests.cpp to always include async_writer - Update thread_system_integration_test.cpp to remove USE_THREAD_SYSTEM conditions - All tests now assume thread_system is a required dependency Part of #216 * refactor(scripts): remove standalone mode options from build.sh - Remove --standalone and --with-thread command line options - Remove STANDALONE_MODE and WITH_THREAD variables - thread_system is now a required dependency Part of #216 * refactor(threading): use std::thread with shared_state for safe lifetime management Replace thread_base with std::thread and shared_ptr to state in log_collector to prevent use-after-free bugs. The shared state pattern ensures worker callback can safely access data even after impl destruction. - Add log_collector_shared_state struct with shared_ptr-managed lifetime - Add worker reset in network_writer destructor for thread safety - Mark IMonitorable integration as Phase 2.2 TODO in examples - Update CHANGELOG with thread_system required dependency details
kcenon
added a commit
that referenced
this pull request
Apr 13, 2026
) * feat: use KCENON feature detection for jthread and source_location - Replace LOGGER_HAS_SOURCE_LOCATION with KCENON_HAS_SOURCE_LOCATION - Replace LOGGER_HAS_JTHREAD with KCENON_HAS_JTHREAD - Include kcenon/common/config/feature_flags.h for unified detection - Keep LOGGER_HAS_* as legacy aliases for backward compatibility Closes #250 * docs: update changelog for KCENON feature detection migration * fix: remove override from deprecated log method for common_system v3.0.0 compatibility The file/line/function overload of log() was removed from common::interfaces::ILogger in common_system v3.0.0 (Issue #217). This method is preserved for backward compatibility but no longer overrides the interface method. This fixes the build error: 'log(...)' marked 'override', but does not override * test: update ilogger_interface_test for common_system v3.0.0 API changes The file/line/function overload of log() was removed from ILogger interface in common_system v3.0.0 (Issue #217). Update test to call the backward-compatible method directly on logger class instead of through ILogger pointer. * docs: update CHANGELOG for common_system v3.0.0 compatibility fix Document the fix for the deprecated log method override issue and test file update. * ci: use thread_system feature branch for KCENON_HAS_COMMON_EXECUTOR fix Update CI workflows to use thread_system feature branch that includes the fix for KCENON_HAS_COMMON_EXECUTOR definition in submodule builds. This resolves the Windows MSVC LNK2019 unresolved external symbol error for thread_pool::is_running() that was caused by inconsistent class layout between logger_system and thread_system builds. Temporary change until thread_system PR is merged to main. * ci: use UNIFIED_USE_LOCAL for local checkout dependencies Enable UNIFIED_USE_LOCAL=ON in CI workflows to use locally checked out common_system and thread_system instead of FetchContent downloading from main branch. This ensures that the thread_system feature branch with KCENON_HAS_COMMON_EXECUTOR fix is used in the build. * docs: update changelog with Windows MSVC linker fix Document the fix for Windows MSVC LNK2019 unresolved external symbol error for thread_pool::is_running(). Root cause: KCENON_HAS_COMMON_EXECUTOR was not defined when thread_system was built as submodule, causing class layout mismatch. Solution: - thread_system core/CMakeLists.txt defines KCENON_HAS_COMMON_EXECUTOR=1 - CI workflows use UNIFIED_USE_LOCAL=ON for local dependencies
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.
Summary
thread_systema required dependency instead of optionalLOGGER_STANDALONE_MODEoption and all standalone mode fallbacksstd::threadtothread_system'sthread_baseChanges
Phase 1: CMake Cleanup
LOGGER_STANDALONE_MODEoption from CMakeLists.txtthread_systemfrom OPTIONAL to REQUIRED dependencyPhase 2: Remove Conditional Compilation
thread_integration_detector.hto always defineUSE_THREAD_SYSTEMLOGGER_STANDALONE_MODEchecks from headersPhase 3: Threading Migration
log_collector.cpp: Uselog_collector_workerclass extendingthread_basenetwork_writer.cpp: Usenetwork_send_workerandnetwork_reconnect_workerbatch_processor.cpp: Usebatch_processing_workerPhase 4: Update Tests
Phase 5: Build Script Cleanup
--standaloneand--with-threadoptions from build.shDocumentation
Breaking Changes
thread_systemis now required - build fails if not found--standaloneoption in build.sh no longer worksthread_systemavailableBenefits
thread_baselifecycle managementthread_systemfeatures always availableTest plan
Closes #216