Keep io context running when stopping test system #4525
Merged
Conversation
This file contains 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
pwojcikdev
force-pushed
the
networking-fixes/io-context-stopping
branch
from
18aa5ae
to
2fd368a
Compare
clemahieu
previously approved these changes
Mar 28, 2024
dsiganos
reviewed
Mar 29, 2024
pwojcikdev
force-pushed
the
networking-fixes/io-context-stopping
branch
from
6479c8e
to
8eb54ba
Compare
dsiganos
reviewed
Mar 29, 2024
| std::shared_ptr<nano::node> nano::test::system::make_disconnected_node (std::optional<nano::node_config> opt_node_config, nano::node_flags flags) | ||
| { | ||
| nano::node_config node_config = opt_node_config.has_value () ? *opt_node_config : default_config (); | ||
| auto node = std::make_shared<nano::node> (io_ctx, nano::unique_path (), node_config, work, flags); | ||
| if (node->init_error ()) | ||
| for (auto i : initialization_blocks) |
There was a problem hiding this comment.
make_disconnected_node() is typically used when someone wants a node without any initialisation and no network connections. the name I gave it isn't ideal, in hindsight, as it implied the node is only disconnected. It replaced code that was constructing nano::node objects without the use of system resources.
So, I think it would be best to not any additional initialisation.
There was a problem hiding this comment.
Thanks for pointing this out. I'll be revisiting the way nodes are added to the system with subsequent io context rework.
dsiganos
approved these changes
Mar 29, 2024
# for free
to join this conversation on GitHub.
Already have an account?
# 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.
Ideally, asio io context would be running through the test lifetime with each component being responsible for properly canceling and joining its async operations. Getting to this point is going to take a bit more work, but this PR is a step towards this goal. It keeps io loop running when stopping nodes, which in turn allows async handlers to complete. Getting it to work required fixing a few lifetime issues and that's also a part of this PR.
Getting this merged will unblock #4523