-
Notifications
You must be signed in to change notification settings - Fork 61
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Cache the Error Before Any Side Effects #619
Conversation
on_error: | ||
on_error:; | ||
/* cache the error code to prevent any potential side effects */ | ||
int cached_error_code = aws_last_error(); | ||
|
||
aws_mem_release(alloc, usable_cpus); | ||
s_aws_event_loop_group_shutdown_sync(el_group); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is one of those throwing some new error because it cant clean up correctly? should we fix that as well?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's code in here that loops through a priority list until it raises the AWS_ERROR_PRIORITY_QUEUE_EMPTY, indicating it's empty.
So it's not an error-as-fail, it's an error-code-as-flow-control
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #619 +/- ##
==========================================
- Coverage 80.09% 80.06% -0.03%
==========================================
Files 28 28
Lines 5943 5945 +2
==========================================
Hits 4760 4760
- Misses 1183 1185 +2 ☔ View full report in Codecov by Sentry. |
@@ -165,12 +165,16 @@ static struct aws_event_loop_group *s_event_loop_group_new( | |||
|
|||
return el_group; | |||
|
|||
on_error: | |||
on_error:; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
; _ ;
Description of changes:
AWS_ERROR_PRIORITY_QUEUE_EMPTY
, when we failed inaws_event_loop_run
while doingpthread_create
.By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.