Fail on Stream Blocked Immediate #4276
Conversation
| @@ -191,8 +193,10 @@ QuicStreamSetReleaseStream( | |||
| uint8_t Flags = (uint8_t)(Stream->ID & STREAM_ID_MASK); | |||
| QUIC_STREAM_TYPE_INFO* Info = &StreamSet->Types[Flags]; | |||
|
|
|||
| CxPlatDispatchLockAcquire(&StreamSet->TypesLock); | |||
There was a problem hiding this comment.
I think this is the only other synchronization we'd need (besides in QuicStreamSetNewLocalStreamID below), but it's a bit tricky.
| Info->CurrentStreamCount++; | ||
| Info->TotalStreamCount++; | ||
| if (!QuicStreamSetInsertStream(StreamSet, Stream)) { | ||
| Status = QUIC_STATUS_OUT_OF_MEMORY; |
There was a problem hiding this comment.
Do we need to roll back the CurrentStreamCount and TotalStreamCount increments on failure? I think we don't we need to fatal the whole connection... But if we do roll back, for the 'fail on blocked' case, we already told the app they're good to go. Rolling back would break that promise. So I think fatal is the best way to go.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #4276 +/- ##
=======================================
Coverage 84.23% 84.23%
=======================================
Files 56 56
Lines 15382 15397 +15
=======================================
+ Hits 12957 12970 +13
- Misses 2425 2427 +2 ☔ View full report in Codecov by Sentry. |
|
FYI, since this is a behavior change for 'fail on block' stream start calls, the down-level tests are failing. Might have to create a new flag to the effect of |
Description
A prototype change to update the
StreamStartAPI to fail inline if the app passesQUIC_STREAM_START_FLAG_FAIL_BLOCKED.Testing
CI/CD
Documentation
TODO