Issue with tl_engine copy constructor exposed in PR #633#646
Merged
Conversation
…ialization - Added a destructor to ConfigurationManager to ensure proper cleanup of JSON configuration objects. - Refactored the constructor to initialize all configuration pointers to nullptr and load configurations from a JSON file. - Updated condition checks in LoadConfFromJSONFile to use logical AND instead of OR for better validation of JSON object types.
- Updated the return values in the parseJsonConf method across multiple configuration files (ChronoGrapher, ChronoKeeper, ChronoPlayer, ChronoVisor) to use the chronolog::CL_SUCCESS constant instead of a hardcoded value of 1 for improved clarity and consistency in success indication. - Adjusted the deployment script to reflect changes in configuration paths for better maintainability.
- Replaced assert statements with conditional checks in the parseJsonConf methods across ChronoGrapher, ChronoKeeper, ChronoPlayer, and ChronoVisor configuration files to improve error handling. - Added error messages to standard output for invalid configuration types, ensuring clearer feedback for configuration issues. - Updated return values to use chl::CL_ERR_INVALID_CONF for consistency in error reporting.
- Added conditional checks in the parseJsonConf methods for CLOCK_CONF and AUTH_CONF to handle parsing errors more gracefully. - Introduced error messages to standard output for invalid configurations, enhancing feedback for users. - Ensured consistent exit behavior on parsing failures by utilizing chronolog::CL_ERR_INVALID_CONF.
… & test/unit/extraction_chain_config
…, ExtractionChain is using variant array instead of argument pack; new KeeperExtractionConfiguration; new KeeperExtractionChain implementation; new GrapherExtractionConfiguration & new GrapherExtractionChain implementation
… and Grapher; added tests/functional/chrono-grapher
… move constructor
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
ibrodkin
changed the title
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
iameneko
reviewed
May 13, 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
2 participants
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.
A combination of conditions compounds to the tl_engine functioning issue mentioned in PR #633 review:
It turns out that std::move(some_object) invokes some_object's copy constructor instead of default move constructor in the case when some_object does not have explicitly defined move constructor
StoryChunkExtractionChain implementation assumes expected move semantics of std:move
invoking thallium ::engine copy constructor might silently finalize the engine and thus prevent normal story chunk transfer in StoryChunkExtractorRDMA & DualEndpointChunkExtractorRDMA.
Changes in this PR :