search

hyperium / h2

HTTP 2.0 client & server implementation for Rust.
MIT License
1.34k stars 269 forks source link

poll_capacity spuriously returns Ready(Some(0)) #628

Closed vadim-eg closed 1 year ago

vadim-eg commented 2 years ago

Sometimes, under active IO poll_capacity returns Poll(Ready(Some(0))). In this situation there is not much caller can do to wait for the capacity becomes available.

It appears to be a concurrency/timing issue - my reconstruction of the events hints that it can happen when wakeup is called from one thread while the outer is sending data on a stream - e.g. -

1) thread A - sender called poll_capacity, got 16K and proceeded to send_data 2) thread B - connection got a window update and sent out some pending frames. capacity did not increase but wakeup was 3) scheduled and send_capacity_inc set. thread A - sent the data and capacity is now 0, send_capacity_inc set 4) thread A - poll_capacity returns 0

One solution is not to wakeup from pop_frame if capacity has not changed. It is not quite clear if this would be a robust solution and this is the only situation - because 'if send_capacity_inc is set - capacity > 0' doesn't appear to be an atomic invariant.

vadim-eg commented 1 year ago

Hello! Is there anybody here? Any thoughts or comments will be appreciated.

seanmonstar commented 1 year ago

I believe the source code even has a TODO comment about this spurious return. I don't know why it occurs, but would welcome investigation!

vadim-eg commented 1 year ago

The summary provides one real scenario how this can happen. We have been running the code with the fix on a scale since then and we haven't seen any traces of the spurious wake ups - so far so good. It would be great though to get it upstream to be able to stick to official releases.

vadim-eg commented 1 year ago

I have opened a new pr with a more comprehensive change and UT that demonstrates the issue.