radicle-dev / radicle-link

The second iteration of the Radicle code collaboration protocol.
Other
421 stars 39 forks source link

io: graceful shutdown #692

Closed kim closed 3 years ago

kim commented 3 years ago

Latest installment of the cancellation saga: the shutdown sequence for an endpoint needs to be carefully grafted in order to not stall blocking tasks (which call back into the async context), and it is advisable to call wait_idle on the endpoint. In turn, our aggressive cancellation policy needs to be loosened a bit to not immediately abort everything, but drain outstanding tasks. Iow, if tests hand on shutdown, it is now a programmer error.

Arguably, the return type of accept is now pretty weird (one future for shutdown, and another one to actually drive the accept loop). Not sure if there is a better formulation, as we are also constrained by opaque types.

Note that the menage tests are still failing, which appears to depend on whether the force-flag-honouring libgit2 is picked up or not.

xla commented 3 years ago

Arguably, the return type of accept is now pretty weird (one future for shutdown, and another one to actually drive the accept loop). Not sure if there is a better formulation, as we are also constrained by opaque types.

From the docs I get that the shutdown shouldn't be polled before accept is complete, with that in mind could the api be hardened by returning the shutdown future in the accept result to ensure this invariant?

kim commented 3 years ago
hardenedNo because the caller wants to cancel the accept loop before it returns (which might be never), **and** the cancellation is a future itself (and there is no async drop). Idk, we could use a tagged union instead of a tulle, but due to opaque types this would really just be about giving the two futures a name.