eio_linux: drain ring on exit #469
Conversation
Normally, all operations should have finished by the time we exit because we don't exit until all fibers have finished, and a fiber can't finish while an operation is in progress. However, this is not the case for cancellation operations, which may still be active in rare cases. So drain any remaining CQEs at exit. Other options here are to allow cancellation operations to block (which they used to do, but that caused other problems), or to have the main operation wait for its cancellation too (but that's tricky and affects the fast-path). Fixes ocaml-multicore#467.
| let rec aux errors = | ||
| if Uring.active_ops uring = 0 then errors | ||
| else ( | ||
| match Uring.wait ~timeout:1.0 uring with |
There was a problem hiding this comment.
I'm not so sure about a 1s timeout here. I could imagine an inflight operation taking longer than that (slow disk or net). Is there any use in having a timeout here at all? It might be better just to leave the process hanging if there's something weird going on with Uring requests not completing. It won't be deadlocked, since in theory the kernel can unwedge the process by pushing a completion event...
There was a problem hiding this comment.
If it's working correctly then the only things that can be in the ring here are cancellation requests for operations that have already finished, which should therefore complete immediately.
If there's anything else, then we'd probably like to see the error rather than hanging (because that means there's a bug in Eio).
There was a problem hiding this comment.
I'm just wary of magic timeout numbers (the 1s). Looping indefinitely with a log seems better. In a stress test, it's kind of hard to spot log entries with errors (although there would be a non-zero exit code here), but quite easy to spot hanging processes taking up all the room on the machine...
|
Closing this in favour of #470. The main loop isn't supposed to finish until |
Normally, all operations should have finished by the time we exit because we don't exit until all fibers have finished, and a fiber can't finish while an operation is in progress. However, this is not the case for cancellation operations, which may still be active in rare cases. So drain any remaining CQEs at exit.
Other options here are to allow cancellation operations to block (which they used to do, but that caused other problems), or to have the main operation wait for its cancellation too (but that's tricky and affects the fast-path).
Fixes #467.