introduce max concurrent streams for stream manager#553
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #553 +/- ##
==========================================
- Coverage 79.76% 79.59% -0.18%
==========================================
Files 28 28
Lines 11893 11933 +40
==========================================
+ Hits 9487 9498 +11
- Misses 2406 2435 +29 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| (void)error; | ||
| sm_connection->connection = NULL; | ||
| } | ||
| AWS_ASSERT(sm_connection->connection != NULL); |
There was a problem hiding this comment.
is that a hard guarantee that connection will always be non-null?
There was a problem hiding this comment.
yeah, that's the only place releasing the connection back to the connection manager and setting this to be null, other than the start destroy.
I can create a helper function to make it more clear. It's the helper function to release the connection back to the manager.
| node = aws_linked_list_next(node); | ||
|
|
||
| /* Verify this connection has no outstanding streams */ | ||
| AWS_FATAL_ASSERT(sm_connection->num_streams_assigned == 0); |
There was a problem hiding this comment.
is this really fatal? can there be any potential case where this occurs in some corner case?
There was a problem hiding this comment.
Not necessary to be fatal, but we should only start to destroy the stream manager when there is no active streams, which is checked before starting the destroy process
3 participants
Issue #, if available:
Description of changes:
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.