Skip to content

refactor: make thread_system a required dependency#217

Merged
kcenon merged 6 commits into
mainfrom
refactor/216-make-thread-system-required
Dec 2, 2025
Merged

refactor: make thread_system a required dependency#217
kcenon merged 6 commits into
mainfrom
refactor/216-make-thread-system-required

Conversation

@kcenon

@kcenon kcenon commented Dec 2, 2025

Copy link
Copy Markdown
Owner

Summary

  • Make thread_system a required dependency instead of optional
  • Remove LOGGER_STANDALONE_MODE option and all standalone mode fallbacks
  • Migrate internal threading from std::thread to thread_system's thread_base
  • Simplify codebase by removing conditional compilation paths

Changes

Phase 1: CMake Cleanup

  • Remove LOGGER_STANDALONE_MODE option from CMakeLists.txt
  • Change thread_system from OPTIONAL to REQUIRED dependency
  • Update LoggerSystemConfig.cmake.in to require ThreadSystem
  • Remove standalone mode conditions from tests and examples CMake

Phase 2: Remove Conditional Compilation

  • Update thread_integration_detector.h to always define USE_THREAD_SYSTEM
  • Remove LOGGER_STANDALONE_MODE checks from headers
  • Remove fallback implementations from adapters
  • Simplify factory classes

Phase 3: Threading Migration

  • log_collector.cpp: Use log_collector_worker class extending thread_base
  • network_writer.cpp: Use network_send_worker and network_reconnect_worker
  • batch_processor.cpp: Use batch_processing_worker

Phase 4: Update Tests

  • Remove standalone test configurations
  • Update integration tests to always use thread_system

Phase 5: Build Script Cleanup

  • Remove --standalone and --with-thread options from build.sh

Documentation

  • Update CHANGELOG.md with breaking changes and migration notes

Breaking Changes

  • thread_system is now required - build fails if not found
  • --standalone option in build.sh no longer works
  • Projects must have thread_system available

Benefits

  • ~90% reduction in conditional compilation
  • Consistent threading via thread_base lifecycle management
  • Single code path instead of standalone + integrated modes
  • Full thread_system features always available

Test plan

  • CMake configuration succeeds with thread_system present
  • Core library (LoggerSystem) builds successfully
  • All existing tests compile and run
  • Build script works without standalone options

Closes #216

- 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
@kcenon kcenon force-pushed the refactor/216-make-thread-system-required branch from edeb770 to 3a0038b Compare December 2, 2025 23:18
@kcenon kcenon merged commit 521ca45 into main Dec 2, 2025
29 checks passed
@kcenon kcenon deleted the refactor/216-make-thread-system-required branch December 2, 2025 23:52
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

refactor: Make thread_system a required dependency (remove standalone mode)

1 participant