feat(iroh-net): Add a Watchable struct for use in the Endpoint API
Description
This implements a Watchable struct and a Watcher which provides access to the watched value in several ways, including some streams.
Breaking Changes
Not yet, adopting it on the API will break things.
Notes & open questions
- Currently I think this is racy, see the TODO items in the code.
- At the moment this emits unused compiler warnings.
Change checklist
- [ ] Self-review.
- [ ] Documentation updates following the style guide, if relevant.
- [ ] Tests if relevant.
- [ ] All breaking changes documented.
Documentation for this PR has been generated and is available at: https://n0-computer.github.io/iroh/pr/2806/docs/iroh/
Last updated: 2024-12-13T09:40:10Z
@flub Any update on this pull request?
@mepi262 the current implementation is racy and needs fixing.
What makes you interested in this?
The reason in which I'm interested is followings.
- I'm very much looking forward to the iroh v1.0.0.
- This pull request is related to the issue, which is contained milestone v1.0.0
- I'm worry about whether this pull request is merged, because this pull request has not been active since last month.
- People forget memory
- Merging pull request is difficult when time has gone, because of risk of conflict.
I wish you good health, good luck and success.
Made some changes:
- Added loom to
watchable.rs(conditionally on theiroh_loomcfg) - Added a loom-based test that was failing in exactly the way @flub predicted
- Fixed the race condition
- Changed
wakersfrom using aRwLockto using aMutex, because we were only usingRwLock::write - Removed
Eitherby makinginitializedsimpler
For anyone who's interested in running the loom test, you get some fun output like this:
LOOM_LOG=trace RUSTFLAGS="--cfg iroh_loom" cargo test --package iroh-net --lib -- util::watchable::tests::test_initialized_always_resolves --exact --nocapture
Should we (well, I) maybe address https://github.com/n0-computer/iroh/discussions/2860#discussioncomment-11239318 in here too?
What's the relationship between Endpoint::conn_type_stream and Connection::remote_address? It seems like remote_address should be deprecated and replaced with a method that returns a ConnectionType based on the current state of the connection.
Should we (well, I) maybe address #2860 (comment) in here too?
What's the relationship between Endpoint::conn_type_stream and Connection::remote_address? It seems like remote_address should be deprecated and replaced with a method that returns a ConnectionType based on the current state of the connection.
I think this PR just introduces the watchable, follow-up PRs can start using it.
Looks good! Any reason this is still draft?
I think this PR just introduces the watchable, follow-up PRs can start using it.
Ah okay. Well my plan was to learn from the refactor of making iroh-net use this new watchable.
And I'm already finding things. E.g. do we want the Watchable to have shutdown functionality.
So, I'm still learning! In any case, I'll split up the PR into watchable introduction & the refactor anyways.
Looks good! Any reason this is still draft?
I think this PR just introduces the watchable, follow-up PRs can start using it.
Ah okay. Well my plan was to learn from the refactor of making iroh-net use this new watchable.
And I'm already finding things. E.g. do we want the Watchable to have
shutdownfunctionality.So, I'm still learning! In any case, I'll split up the PR into watchable introduction & the refactor anyways.
Makes sense as well. No big deal if you'd like to do this all in one place.
Netsim report & logs for this PR have been generated and is available at: LOGS This report will remain available for 3 days.
Last updated for commit: 5066045