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

experiment: try to only use tokio::mpsc in iroh-net #2614

Closed
wants to merge 3 commits into from

Conversation

dignifiedquire
Copy link
Contributor

I am failing to implement the send_blocking, if anyone has ideas, I would love to hear them

@rklaehn
Copy link
Contributor

rklaehn commented Aug 13, 2024

Pushed a fix. But this isn't really send_blocking, more like send_blocking without runtime and send_detached or whatever in case a runtime is found.

It might be that this is OK in most cases. But then the fn should be called send_sync or something and return just unit to signal that it is fire and forget / best effort.

Copy link

github-actions bot commented Aug 13, 2024

Documentation for this PR has been generated and is available at: https://n0-computer.github.io/iroh/pr/2614/docs/iroh/

Last updated: 2024-08-13T10:00:22Z

@rklaehn
Copy link
Contributor

rklaehn commented Aug 13, 2024

As you mention, the calls to send_blocking can be replaced by a normal send call just by making some fns async. So should we just do that?

send_blocking works now, except that it is not really blocking in the case where a runtime is found. But we don't need it at all in iroh-net.

@dignifiedquire
Copy link
Contributor Author

As you mention, the calls to send_blocking can be replaced by a normal send call just by making some fns async. So should we just do that?

yes, we should try and do that

@dignifiedquire
Copy link
Contributor Author

closing in favor of #2620

github-merge-queue bot pushed a commit that referenced this pull request Aug 14, 2024
## Description

Removes async-channel from iroh-net and replaces it with the tokio mpsc
channel.

This is a mergeable version of
#2614

## Breaking Changes

LocalSwarmDiscovery is no longer UnwindSafe

## Notes & open questions

Open question: I am using blocking_send from inside the windows
RouteMonitor. This depends on that the callback thread is not a tokio
thread. I have no idea if this is a reasonable assumption to make.
Otherwise we would have to bring back the runtime check condition or
just capture a runtime handle and spawn a task.

Note: what even is this? The only message there is is
NetworkMessage::Change, and there is no timestamp or anything. So could
I just use try_send? If the queue is full, there is already a Change in
it, so it is as good as the new one...

## Change checklist

- [x] Self-review.
- [ ] Documentation updates following the [style
guide](https://rust-lang.github.io/rfcs/1574-more-api-documentation-conventions.html#appendix-a-full-conventions-text),
if relevant.
- [ ] Tests if relevant.
- [ ] All breaking changes documented.

---------

Co-authored-by: dignifiedquire <me@dignifiedquire.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants