Removes AWS_FATAL_ASSERT in s_aws_event_loop_group_shutdown_async #515
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
waahm7
commented
Sep 6, 2022
| @@ -63,16 +63,14 @@ static void s_aws_event_loop_group_shutdown_async(struct aws_event_loop_group *e | |||
| struct aws_thread cleanup_thread; | |||
| AWS_ZERO_STRUCT(cleanup_thread); | |||
|
|
|||
| AWS_FATAL_ASSERT(aws_thread_init(&cleanup_thread, el_group->allocator) == AWS_OP_SUCCESS); | |||
| aws_thread_init(&cleanup_thread, el_group->allocator); | |||
There was a problem hiding this comment.
This always return Success.
waahm7
commented
Sep 6, 2022
| AWS_FATAL_ASSERT( | ||
| aws_thread_launch(&cleanup_thread, s_event_loop_destroy_async_thread_fn, el_group, &thread_options) == | ||
| AWS_OP_SUCCESS); | ||
| aws_thread_launch(&cleanup_thread, s_event_loop_destroy_async_thread_fn, el_group, &thread_options); |
There was a problem hiding this comment.
This can only fail if we cannot create a thread. As the function is void, we cann’t return an AWS_OP_ERR without changing the return type. We use this function as a completion call back during aws_ref_count_init at https://github.com/awslabs/aws-c-io/blob/main/source/event_loop.c#L116 which expects the return type to be void.
There was a problem hiding this comment.
If this fails and shutdown is blocking (aws_thread_join_all_managed is used without a timeout) then it will be an indefinite wait.
bretambrose
approved these changes
Sep 6, 2022
2 tasks
1 task
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Issue #, if available: aws/aws-sdk-cpp#1809
Description of changes:
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.