Fix crash when aws_s3_meta_request has function called on it after completion #217
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
…increment_read_window() was called afte the meta-request completed
…Don't release the client until meta-request is destroyed.
DmitriyMusatkin
approved these changes
Oct 7, 2022
grrtrr
pushed a commit
to grrtrr/aws-c-s3
that referenced
this pull request
Nov 1, 2022
…mpletion (awslabs#217) **Issue**: If a function was called on a meta-request after it completed, this would lead to a crash (this affected `aws_s3_meta_request_increment_read_window()`, `aws_s3_meta_request_cancel()`, and `aws_s3_meta_request_pause()`) Since these functions might be called from different threads than the thread that completes the meta-request, it would be nearly impossible for a customer to avoid these crashes. **Description of changes:** The reason for the crash was that the `meta_request->client` pointer would get cleared when the meta-request completed. To fix this bug, we stop clearing this pointer early, and keep`meta_request->client` pointer valid for the lifetime of the meta-request. We evaluated doing something much more complex (checking state and toying with reference counts before scheduling work on the client) but it seemed finicky and error-prone.
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 a function was called on a meta-request after it completed, this would lead to a crash (this affected
aws_s3_meta_request_increment_read_window(),aws_s3_meta_request_cancel(), andaws_s3_meta_request_pause())Since these functions might be called from different threads than the thread that completes the meta-request, it would be nearly impossible for a customer to avoid these crashes.
Description of changes:
The reason for the crash was that the
meta_request->clientpointer would get cleared when the meta-request completed.To fix this bug, we stop clearing this pointer early, and keep
meta_request->clientpointer valid for the lifetime of the meta-request.We evaluated doing something much more complex (checking state and toying with reference counts before scheduling work on the client) but it seemed finicky and error-prone.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.