ref(iroh-net)!: Make Endpoint::close not consume self

[iacore]

Description

The endpoint can be freely cloned, so consuming self on shutdown is somewhat fruitless. This changes it to take &self like a normal method.

Breaking Changes

• Endpoint::close now takes &self rather than self. This can in some situations mean an existing (clone of an) endpoint might be dropped too early as a temporary variable.

Notes & open questions

Change checklist

• Self-review.
• Documentation updates following the style guide, if relevant.
• Tests if relevant.
• All breaking changes documented.

[flub]

This needs to get the changes in main, and then check if the new doc comment needs tweaking because we need to make it clear how this works. But in general I think this is probably a good change.

[flub]

LGTM to me, but would love @divagant-martian to lgtm too (or not)

[divagant-martian]

this was in part because of how quinn used to work. Closing the magic socket before dropping the endpoint would basically always emit an error in logs breaking all our cli tests. The changes in the endpoint loop make this less likely to happen in this scenario. This is not to say the previous state was exactly what we needed: As the comment mentions, it's there to make it less likely that the socket is polled. In general closing here is messy and kinda gross. When we wrote this code (not sure now) the socket was the last resource the (quinn) endpoint dropped. This was later that the drop of the endpoint itself given the many cloned Arcs and spawned tasks.

All this long rant is to say: there was a reason this looked this way. To get a best effort imperfect close because both how quinn works and the lack of async drop (and I thought the rant was over) but after we updated quinn I think the error is no longer an issue and we are fine with dropping the endpoint after the socket in most cases

[flub]

Thanks @iacore for pushing this change!