Skip to content
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

Fix shutdown_callback or returning NULL contract for meta_request #440

Merged
merged 18 commits into from
Jun 20, 2024

Conversation

waahm7
Copy link
Contributor

@waahm7 waahm7 commented Jun 18, 2024

Issue #, if available:
aws/aws-sdk-java-v2#5277

Description of changes:
There are multiple places where a meta-request can fail after doing the aws_s3_meta_request_init_base like

if (aws_s3_parse_request_range_header(
,
if (!client->s3express_provider) {
,
if (endpoint == NULL) {
. In those cases, we were triggering the shutdown callback as well as returning NULL, which is a crime and can lead to double-free errors. This PR moves the shutdown_callback assignment to be the last thing set on the meta_request so that we don’t trigger the shutdown_callback if we fail for any reason after creating the meta_request and before returning it to customers.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@waahm7 waahm7 changed the title WIP | shutdown callback fix Fix shutdown_callback or returning NULL contract for meta_request Jun 18, 2024
@waahm7 waahm7 marked this pull request as ready for review June 18, 2024 21:36
Copy link
Contributor

@graebm graebm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good overall, but had questions

source/s3_client.c Outdated Show resolved Hide resolved
source/s3_client.c Show resolved Hide resolved
Co-authored-by: Michael Graeb <graebm@amazon.com>
@waahm7 waahm7 merged commit a549581 into main Jun 20, 2024
30 checks passed
@waahm7 waahm7 deleted the fix-null-and-shutdown-callback branch June 20, 2024 19:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants