Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use coroutines in tcp listener #4581

Conversation

pwojcikdev
Copy link
Contributor

@pwojcikdev pwojcikdev commented Apr 22, 2024

To fix remaining TSAN issues this PR changes tcp listener implementation to use coroutines with a strand. The resulting code is structurally almost identical to the threaded version.

Continuation of #4523

clemahieu
clemahieu previously approved these changes Apr 23, 2024

/**
* A cancellation signal that can be emitted from any thread.
* I follows the same semantics as asio::cancellation_signal.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • I follows

Type-o here.

acceptor.listen (asio::socket_base::max_listen_connections);

{
std::lock_guard<nano::mutex> lock{ mutex };
local = acceptor.local_endpoint ();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this need to be strand protected?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Up to this point (including local_endpoint call) all operations are synchronous, so there should be no need to wrap it in a strand. It would be necessary if it was called after main loop is started, that's why it's cached beforehand.

@pwojcikdev pwojcikdev merged commit 368d8e2 into nanocurrency:develop Apr 24, 2024
26 checks passed
@qwahzi qwahzi added this to the V27 milestone May 21, 2024
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.

3 participants