self.pending_capacity.is_empty() not always guaranteed

[DarrenTsung]

https://github.com/carllerche/h2/blob/master/src/proto/streams/prioritize.rs#L357

I'm getting panics at this assert when trying to load test by sending 4k notifications as fast as possible. Looking at the code, it looks safe to have this assert as try_assign_capacity can push 0 or 1 streams into self.pending_capacity and can be called from assign_connection_capacity which must pop 1 stream from self.pending_capacity before calling try_assign_capacity.

However, try_assign_capacity can also be called from send_data (https://github.com/carllerche/h2/blob/master/src/proto/streams/prioritize.rs#L148).

When opening a bunch of simultaneous streams, I seeing send_data getting called multiple times (unable to assign capacity, the streams are put in self.pending_capacity). Then a call to assign_connection_capacity when there is extra capacity, then the panic because there are still streams waiting in self.pending_capacity.

Do you guys know what might be happening? Maybe there's a bug externally to the assert and the assert should still hold true, or maybe this assert doesn't hold true with code pushed after the fact?

Thanks!

[seanmonstar]

So, a bunch of streams are waiting for capacity, and then when a WINDOW_UPDATE is received for the connection, a pending stream is popped from the queue, and try_assign_capacity is called with, while there are still other connections pending?

That seems like the assertion is incorrect then. @carllerche?

[carllerche]

Yeah, that assertion does not seem correct. I'm not sure what I was thinking originally. That debug_assert was included in the original PR so I was probably still trying to figure out the guarantees and adding as many assertions as possible to see what would fail!

It would be fine to remove I think.

[carllerche]

It would also be nice to include a test that demonstrates the panic as it this also is indicative of test coverage lacking. 👍

[DarrenTsung]

Oh, just read your message, I'll write up a test for that.