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

Indirection/abstraction over tokio::spawn and friends #2979

Open
matheus23 opened this issue Nov 29, 2024 · 4 comments
Open

Indirection/abstraction over tokio::spawn and friends #2979

matheus23 opened this issue Nov 29, 2024 · 4 comments
Milestone

Comments

@matheus23
Copy link
Contributor

matheus23 commented Nov 29, 2024

Iroh's use of tokio is very particular/opinionated.

  • No use of tokio::spawn without wrapping the JoinHandle with AbortOnDropHandle
  • No use of JoinSet::spawn without some loop somewhere calling tasks.join_next().await, if !tasks.is_empty() (to avoid leaking memory from task results accumulating) (EDIT: this might only be necessary if used with else inside tokio::select!, see refactor(iroh, iroh-relay): JoinSet disabling in tokio::select! #3052 )
  • Every spawn must use an .instrument() for tracing.

Furthermore, for iroh in the browser (#2799) we will need to abstract away all tokio::spawn usage because it's not available in the browser! (I've already implemented a JoinSet alternative on top of wasm_bindgen_futures for exactly that reason)

Given that such an indirection would have to be created for the browser work anyways, this might be a good opportunity to introduce an API at that indirection that avoids all of the above footguns (forgetting .instrument(), leaking tasks, leaking memory).

@rklaehn
Copy link
Contributor

rklaehn commented Nov 29, 2024

Iroh's use of tokio is very particular/opinionated

sane is the word you are looking for...

@matheus23 matheus23 added this to the v1.0.0 milestone Dec 13, 2024
@matheus23
Copy link
Contributor Author

Perhaps worth keeping in mind that AbortOnDropHandle isn't the be-all end-all of task leaking: n0-computer/iroh-gossip#20

I highly doubt it but perhaps we should spend a couple of minutes thinking about if there's a good solution for this problem once we get to this issue.

@TotalKrill
Copy link

From what I can see this is now the last issue to get webassembly support for iroh-net as outlined in #2799

Is there a plan to do this? :)

@matheus23
Copy link
Contributor Author

@TotalKrill happy to see people so interested in this :)

As I wrote in the issue:

This is a non-exhaustive list of things that need to land for Wasm support:

So even if this lands, there will be more work required. It's just us keeping track of things to make sure we have a rough overview.

But to answer the actual question, yes, you can see the module that implements this indirection in the matheus23/iroh-browser branch here:

If you want to, you're free to depend on this branch yourself and start to play with iroh in browsers - it works with the default public relays we have deployed as well. You can see me playing and testing that branch over here as well: https://github.com/matheus23/browser-iroh-net-demo/blob/update/Cargo.toml

Possibly very soon we'll have a demo branch that's easier to set up for all folks interested to play with.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: No status
Development

No branches or pull requests

3 participants