search

tokio-rs / io-uring

The `io_uring` library for Rust
Apache License 2.0
1.19k stars 131 forks source link

Add support for IORING_REGISTER_SYNC_CANCEL #225

Closed gardnervickers closed 1 year ago

gardnervickers commented 1 year ago

Adds a new Submitter method, register_sync_cancel which can be used to synchronously cancel operations. Additionally, this adds AsyncCancelFlags used to configure cancellation.

I didn't integrate the AsyncCancelFlags with the opcode::AsyncCancel in this PR, mostly because I haven't needed it but I can either do that in this PR or a follow-up PR.

gardnervickers commented 1 year ago

Thanks for the review @quininer and @FrankReh. I pushed a new set of changes which include a builder for constructing match criteria for the register_sync_cancel call.

The main motivation for it was that taking Option<impl UseFixed> in register_sync_cancel would require users to specify a concrete type even if they supply None for an FD. With the MatchOn builder, we can take impl UseFixed and just extract the sealed::Target from it.

Additionally, I think the MatchOn builder makes it a bit easier to supply the correct set of arguments, since users don't set the cancellation flags directly now. Please let me know what you think, I wasn't sure if it was adding too much extra complexity for this crate or not.

FrankReh commented 1 year ago

There may still be some issues outstanding but I wanted to say the model you chose for the unit test and the use of stdin to have operations blocked on is very nice. I don't remember seeing that in the unit tests before. I suspect it will be copied for other unit tests down the line.

gardnervickers commented 1 year ago

There may still be some issues outstanding but I wanted to say the model you chose for the unit test and the use of stdin to have operations blocked on is very nice. I don't remember seeing that in the unit tests before. I suspect it will be copied for other unit tests down the line.

Thanks, I actually ended up switching away from using stdin in favor of creating an eventfd, partly because I was worried about using a shared resource in the tests, and partly because I wasn't sure if there would be a problem using stdin with fixed files.

quininer commented 1 year ago

LGTM. I will release 0.6.0 on wednesday.