Conversation
|
Documentation for this PR has been generated and is available at: https://n0-computer.github.io/iroh/pr/4091/docs/iroh/ Last updated: 2026-04-09T11:23:26Z |
matheus23
left a comment
There was a problem hiding this comment.
I don't understand where we call Endpoint::close in this version.
We should be calling Endpoint::close to ensure we actually properly send the close frame.
With the current version this kinda works out because you can "wait" for the conn.closed().await on the other side "out-of-band" via the barrier as the communication mechanism.
But we don't need the barrier at all, if we just endpoint.close().await on both sides of Pair::run.
You seem to intentionally avoid doing this, what's the reasoning for that?
We don't. We also don't on main. I removed it after a suggestion from @flub that we don't need it, because we are not testing for closing behavior but for holepunching here. Removing the endpoint close makes the tests run faster, too. All tests await I can change it back to calling |
I see. I intentionally added it back, thinking "we don't close properly, so obviously this will take longer than necessary". The idea being here that if we don't close properly, we might not even send the close message, leaving the other side hanging. Can you add a comment next the barrier use, essentially explaining that we use this as an alternative mechanism to |
|
FWIW I'd love those patchbay close tests on noq ;) |
Description
Should make #4086 obsolete, I think