Dispatch Queue Shutdown Polish #708
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## grand_dispatch_queue #708 +/- ##
========================================================
+ Coverage 79.59% 79.60% +0.01%
========================================================
Files 30 30
Lines 6126 6139 +13
========================================================
+ Hits 4876 4887 +11
- Misses 1250 1252 +2 ☔ View full report in Codecov by Sentry. |
| s_dispatch_loop_release(dispatch_loop); | ||
|
|
||
| s_lock_synced_data(dispatch_loop); | ||
| aws_condition_variable_wait_pred( |
There was a problem hiding this comment.
Trivial: May be worth leaving a comment here as there's kind of a lot going on in this one function.
THIS thread is blocked at this point until the dispatch loop hits zero refs.
synced lock is held but is released by aws_condition_variable_wait_pred to allow the lock to be held by the dispatch_loop's event_loop.
synced lock is held again before the aws_condition_variable_wait_pred continues at which point we unlock after.
Or not. Mainly had to dig around a bit to make sure we weren't gonna deadlock here as underlying pthread_cond_wait() doesn't give much info.
There was a problem hiding this comment.
Well, we use the condition-variable-and-mutex combo all over the codebase (mostly in tests) so I don't think it's critical to explain a single instance of it.
| * dispatch queue alive unnecessarily long, even after aws_event_loop and aws_dispatch_loop have been fully | ||
| * destroyed and cleaned up. To mitigate this, we ensure an iteration is scheduled no longer than 1 second in the | ||
| * dispatch queue alive unnecessarily long, which blocks event loop group shutdown from completion. | ||
| * To mitigate this, we ensure an iteration is scheduled no longer than 1 second in the |
There was a problem hiding this comment.
Trivial: Maybe change this comment to clamping to "AWS_DISPATCH_QUEUE_MAX_FUTURE_SERVICE_INTERVAL" since it's possible we change the time amount in the future?
Refactors dispatch queue shutdown so that no loop-associated memory is unreleased at the time event loop group destroy completes.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.