DataDog / glommio

Glommio is a thread-per-core crate that makes writing highly parallel asynchronous applications in a thread-per-core architecture easier for rustaceans.
Other
2.93k stars 161 forks source link

Improve Gate::close to support repeated calls #662

Closed vlovich closed 1 month ago

vlovich commented 2 months ago

What does this PR do?

If Gate::close has a timeout, closing the gate is left in a dangling state with no way to safely resume waiting. This change makes it safe in that calling close on a closing Gate still waits for any outstanding tasks. This way you can repeatedly try calling gate close with a timeout set while safely waiting for all tasks to complete.

This is technically an observable change in that a previous call to close when still in "closing" would have returned an error but now waits for the currently running tasks instead. I can't imagine anyone actually relies too much on this.

Motivation

I have a gate that protects whether or not I've started doing I/O. My close routine awaits the gate closure. However, there's a risk that I have a timeout hit that cancels the waiting for close. If I try a subsequent close, it'll have no choice but to skip calling close even though there may be tasks active, resulting in entering a piece of code unexpectedly that the close was supposed to protect.

With this change, I have a way to now force that the gate is always in a close state regardless of timeouts by invoking close if !is_closed.

Related issues

Additional Notes

Checklist

[X] I have added unit tests to the code I am submitting [X] My unit tests cover both failure and success scenarios [] If applicable, I have discussed my architecture

vlovich commented 1 month ago

Updated the PR to roll in another enhancement I've wanted, which is for Gate close to be synchronous & return an asynchronous for the waiter. That way Gate::close is guaranteed to apply before the future future is awaited. It's useful for this closure pattern where:

let was_open = gate.is_open();
let closure = gate.close();
if was_open { 
    // This is the only dirty state possible because any straggling I/O are guaranteed to see the gate as closing
    // and will return an error.
    complete_outstanding_work().await?;
}
closure.await? // We're now guaranteed that there's no outstanding writes.

This might be a breaking change if anyone relied on the closure not applying until the future awaited so this would warrant a bump to 0.10 for the next release & release notes.