Skip to content

Commit 9054af0

Browse files
sbisciglsbiscigl
committed
fix cancel s3crt request callback to not outlive request lifetime
1 parent 1e6b979 commit 9054af0

File tree

7 files changed

+71
-20
lines changed

7 files changed

+71
-20
lines changed

‎generated/src/aws-cpp-sdk-s3-crt/include/aws/s3-crt/S3CrtClient.h‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8146,6 +8146,9 @@ class AWS_S3CRT_API S3CrtClient : public Aws::Client::AWSXMLClient, public Aws::
81468146
std::shared_ptr<Aws::Http::HttpResponse> response;
81478147
std::shared_ptr<Aws::Crt::Http::HttpRequest> crtHttpRequest;
81488148
Aws::UniquePtr<struct aws_s3_checksum_config> checksumConfig;
8149+
std::mutex requestLifetimeMutex;
8150+
std::condition_variable requestLifetimeCondition;
8151+
bool requestLifetimeShouldContinue = true;
81498152
};
81508153

81518154
Aws::Client::XmlOutcome GenerateXmlOutcome(const std::shared_ptr<Http::HttpResponse>& response) const;
@@ -8167,7 +8170,7 @@ class AWS_S3CRT_API S3CrtClient : public Aws::Client::AWSXMLClient, public Aws::
81678170
};
81688171

81698172
static void CrtClientShutdownCallback(void* data);
8170-
void CancelCrtRequestAsync(aws_s3_meta_request* meta_request) const;
8173+
void CancelCrtRequestAsync(aws_s3_meta_request* meta_request, S3CrtClient::CrtRequestCallbackUserData* userData) const;
81718174
static int S3CrtRequestHeadersCallback(aws_s3_meta_request* meta_request, const struct aws_http_headers* headers, int response_status,
81728175
void* user_data);
81738176
static int S3CrtRequestGetBodyCallback(struct aws_s3_meta_request* meta_request, const struct aws_byte_cursor* body, uint64_t range_start,

‎generated/src/aws-cpp-sdk-s3-crt/source/S3CrtClient.cpp‎

Lines changed: 21 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -165,6 +165,8 @@ const char ALLOCATION_TAG[] = "S3CrtClient";
165165
const char* S3CrtClient::GetServiceName() { return SERVICE_NAME; }
166166
const char* S3CrtClient::GetAllocationTag() { return ALLOCATION_TAG; }
167167

168+
const std::chrono::minutes REQUEST_LIFETIME_WAIT_TIMEOUT = std::chrono::minutes(1);
169+
168170
S3CrtClient::S3CrtClient(const S3CrtClient& rhs)
169171
: BASECLASS(rhs.m_clientConfiguration,
170172
Aws::MakeShared<Aws::Auth::S3ExpressSignerProvider>(
@@ -501,9 +503,17 @@ void S3CrtClient::CrtClientShutdownCallback(void* data) {
501503
wrappedData->clientShutdownSem->Release();
502504
}
503505

504-
void S3CrtClient::CancelCrtRequestAsync(aws_s3_meta_request* meta_request) const {
506+
void S3CrtClient::CancelCrtRequestAsync(aws_s3_meta_request* meta_request, S3CrtClient::CrtRequestCallbackUserData* userData) const {
505507
assert(meta_request);
506-
m_clientConfiguration.executor->Submit([meta_request]() { aws_s3_meta_request_cancel(meta_request); });
508+
userData->requestLifetimeShouldContinue = false;
509+
m_clientConfiguration.executor->Submit([meta_request, userData]() {
510+
{
511+
std::lock_guard<std::mutex> requestLifetimeLock{userData->requestLifetimeMutex};
512+
aws_s3_meta_request_cancel(meta_request);
513+
userData->requestLifetimeShouldContinue = true;
514+
}
515+
userData->requestLifetimeCondition.notify_all();
516+
});
507517
}
508518

509519
int S3CrtClient::S3CrtRequestHeadersCallback(struct aws_s3_meta_request* meta_request, const struct aws_http_headers* headers,
@@ -525,7 +535,7 @@ int S3CrtClient::S3CrtRequestHeadersCallback(struct aws_s3_meta_request* meta_re
525535
auto& shouldContinueFn = userData->originalRequest->GetContinueRequestHandler();
526536
const HttpRequest* httpRequest = userData->request ? userData->request.get() : nullptr;
527537
if (shouldContinueFn && !shouldContinueFn(httpRequest)) {
528-
userData->s3CrtClient->CancelCrtRequestAsync(meta_request);
538+
userData->s3CrtClient->CancelCrtRequestAsync(meta_request, userData);
529539
}
530540

531541
return AWS_OP_SUCCESS;
@@ -558,7 +568,7 @@ int S3CrtClient::S3CrtRequestGetBodyCallback(struct aws_s3_meta_request* meta_re
558568
auto& shouldContinueFn = userData->originalRequest->GetContinueRequestHandler();
559569
const HttpRequest* httpRequest = userData->request ? userData->request.get() : nullptr;
560570
if (shouldContinueFn && !shouldContinueFn(httpRequest)) {
561-
userData->s3CrtClient->CancelCrtRequestAsync(meta_request);
571+
userData->s3CrtClient->CancelCrtRequestAsync(meta_request, userData);
562572
}
563573

564574
return AWS_OP_SUCCESS;
@@ -580,7 +590,7 @@ void S3CrtClient::S3CrtRequestProgressCallback(struct aws_s3_meta_request* meta_
580590
auto& shouldContinueFn = userData->originalRequest->GetContinueRequestHandler();
581591
const HttpRequest* httpRequest = userData->request ? userData->request.get() : nullptr;
582592
if (shouldContinueFn && !shouldContinueFn(httpRequest)) {
583-
userData->s3CrtClient->CancelCrtRequestAsync(meta_request);
593+
userData->s3CrtClient->CancelCrtRequestAsync(meta_request, userData);
584594
}
585595

586596
return;
@@ -643,6 +653,12 @@ void S3CrtClient::S3CrtRequestFinishCallback(struct aws_s3_meta_request* meta_re
643653
AWS_UNREFERENCED_PARAM(meta_request);
644654
auto* userData = static_cast<S3CrtClient::CrtRequestCallbackUserData*>(user_data);
645655

656+
{
657+
std::unique_lock<std::mutex> requestLifetimeLock{userData->requestLifetimeMutex};
658+
userData->requestLifetimeCondition.wait_for(requestLifetimeLock, REQUEST_LIFETIME_WAIT_TIMEOUT,
659+
[userData]() -> bool { return userData->requestLifetimeShouldContinue; });
660+
}
661+
646662
if (meta_request_result->error_code != AWS_ERROR_SUCCESS && meta_request_result->response_status == 0) {
647663
/* client side error */
648664
userData->response->SetClientErrorType(CoreErrors::NETWORK_CONNECTION);

‎tools/code-generation/generator/src/main/resources/com/amazonaws/util/awsclientgenerator/velocity/cpp/s3/S3ClientHeader.vm‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -207,6 +207,9 @@ namespace ${rootNamespace}
207207
std::shared_ptr<Aws::Http::HttpResponse> response;
208208
std::shared_ptr<Aws::Crt::Http::HttpRequest> crtHttpRequest;
209209
Aws::UniquePtr<struct aws_s3_checksum_config> checksumConfig;
210+
std::mutex requestLifetimeMutex;
211+
std::condition_variable requestLifetimeCondition;
212+
bool requestLifetimeShouldContinue = true;
210213
};
211214

212215
Aws::Client::XmlOutcome GenerateXmlOutcome(const std::shared_ptr<Http::HttpResponse>& response) const;
@@ -230,7 +233,7 @@ namespace ${rootNamespace}
230233
};
231234

232235
static void CrtClientShutdownCallback(void *data);
233-
void CancelCrtRequestAsync(aws_s3_meta_request *meta_request) const;
236+
void CancelCrtRequestAsync(aws_s3_meta_request *meta_request, S3CrtClient::CrtRequestCallbackUserData* userData) const;
234237
static int S3CrtRequestHeadersCallback(aws_s3_meta_request *meta_request, const struct aws_http_headers *headers, int response_status, void *user_data);
235238
static int S3CrtRequestGetBodyCallback(struct aws_s3_meta_request *meta_request, const struct aws_byte_cursor *body, uint64_t range_start, void *user_data);
236239
static void S3CrtRequestProgressCallback(struct aws_s3_meta_request *meta_request, const struct aws_s3_meta_request_progress *progress, void *user_data);

‎tools/code-generation/generator/src/main/resources/com/amazonaws/util/awsclientgenerator/velocity/cpp/s3/SmithyS3ClientHeader.vm‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -231,6 +231,9 @@ namespace ${rootNamespace}
231231
std::shared_ptr<Aws::Http::HttpResponse> response;
232232
std::shared_ptr<Aws::Crt::Http::HttpRequest> crtHttpRequest;
233233
Aws::UniquePtr<struct aws_s3_checksum_config> checksumConfig;
234+
std::mutex requestLifetimeMutex;
235+
std::condition_variable requestLifetimeCondition;
236+
bool requestLifetimeShouldContinue = true;
234237
};
235238

236239
Aws::Client::XmlOutcome GenerateXmlOutcome(const std::shared_ptr<Http::HttpResponse>& response) const;
@@ -250,7 +253,7 @@ namespace ${rootNamespace}
250253
};
251254

252255
static void CrtClientShutdownCallback(void *data);
253-
void CancelCrtRequestAsync(aws_s3_meta_request *meta_request) const;
256+
void CancelCrtRequestAsync(aws_s3_meta_request *meta_request, S3CrtClient::CrtRequestCallbackUserData* userData) const;
254257
static int S3CrtRequestHeadersCallback(aws_s3_meta_request *meta_request, const struct aws_http_headers *headers, int response_status, void *user_data);
255258
static int S3CrtRequestGetBodyCallback(struct aws_s3_meta_request *meta_request, const struct aws_byte_cursor *body, uint64_t range_start, void *user_data);
256259
static void S3CrtRequestProgressCallback(struct aws_s3_meta_request *meta_request, const struct aws_s3_meta_request_progress *progress, void *user_data);

‎tools/code-generation/generator/src/main/resources/com/amazonaws/util/awsclientgenerator/velocity/cpp/s3/s3-crt/S3CrtServiceClientSourceInit.vm‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,8 @@
5555
#set($hasEventStreamInputOperation = true)
5656
#end
5757
#end
58+
const std::chrono::minutes REQUEST_LIFETIME_WAIT_TIMEOUT = std::chrono::minutes(1);
59+
5860
${className}::${className}(const ${className} &rhs) :
5961
BASECLASS(rhs.m_clientConfiguration,
6062
Aws::MakeShared<Aws::Auth::S3ExpressSignerProvider>(ALLOCATION_TAG,

‎tools/code-generation/generator/src/main/resources/com/amazonaws/util/awsclientgenerator/velocity/cpp/s3/s3-crt/S3CrtSpecificOperations.vm‎

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -11,10 +11,16 @@ void S3CrtClient::CrtClientShutdownCallback(void *data)
1111
wrappedData->clientShutdownSem->Release();
1212
}
1313

14-
void S3CrtClient::CancelCrtRequestAsync(aws_s3_meta_request *meta_request) const {
14+
void S3CrtClient::CancelCrtRequestAsync(aws_s3_meta_request *meta_request, S3CrtClient::CrtRequestCallbackUserData* userData) const {
1515
assert(meta_request);
16-
m_clientConfiguration.executor->Submit([meta_request]() {
17-
aws_s3_meta_request_cancel(meta_request);
16+
userData->requestLifetimeShouldContinue = false;
17+
m_clientConfiguration.executor->Submit([meta_request, userData]() {
18+
{
19+
std::lock_guard<std::mutex> requestLifetimeLock{userData->requestLifetimeMutex};
20+
aws_s3_meta_request_cancel(meta_request);
21+
userData->requestLifetimeShouldContinue = true;
22+
}
23+
userData->requestLifetimeCondition.notify_all();
1824
});
1925
}
2026

@@ -38,7 +44,7 @@ int S3CrtClient::S3CrtRequestHeadersCallback(struct aws_s3_meta_request *meta_re
3844
auto& shouldContinueFn = userData->originalRequest->GetContinueRequestHandler();
3945
const HttpRequest* httpRequest = userData->request ? userData->request.get() : nullptr;
4046
if (shouldContinueFn && !shouldContinueFn(httpRequest)) {
41-
userData->s3CrtClient->CancelCrtRequestAsync(meta_request);
47+
userData->s3CrtClient->CancelCrtRequestAsync(meta_request, userData);
4248
}
4349

4450
return AWS_OP_SUCCESS;
@@ -73,7 +79,7 @@ int S3CrtClient::S3CrtRequestGetBodyCallback(struct aws_s3_meta_request *meta_re
7379
auto& shouldContinueFn = userData->originalRequest->GetContinueRequestHandler();
7480
const HttpRequest* httpRequest = userData->request ? userData->request.get() : nullptr;
7581
if (shouldContinueFn && !shouldContinueFn(httpRequest)) {
76-
userData->s3CrtClient->CancelCrtRequestAsync(meta_request);
82+
userData->s3CrtClient->CancelCrtRequestAsync(meta_request, userData);
7783
}
7884

7985
return AWS_OP_SUCCESS;
@@ -95,7 +101,7 @@ void S3CrtClient::S3CrtRequestProgressCallback(struct aws_s3_meta_request *meta_
95101
auto& shouldContinueFn = userData->originalRequest->GetContinueRequestHandler();
96102
const HttpRequest* httpRequest = userData->request ? userData->request.get() : nullptr;
97103
if (shouldContinueFn && !shouldContinueFn(httpRequest)) {
98-
userData->s3CrtClient->CancelCrtRequestAsync(meta_request);
104+
userData->s3CrtClient->CancelCrtRequestAsync(meta_request, userData);
99105
}
100106

101107
return;
@@ -159,6 +165,12 @@ void S3CrtClient::S3CrtRequestFinishCallback(struct aws_s3_meta_request *meta_re
159165
AWS_UNREFERENCED_PARAM(meta_request);
160166
auto *userData = static_cast<S3CrtClient::CrtRequestCallbackUserData*>(user_data);
161167

168+
{
169+
std::unique_lock<std::mutex> requestLifetimeLock{userData->requestLifetimeMutex};
170+
userData->requestLifetimeCondition.wait_for(requestLifetimeLock, REQUEST_LIFETIME_WAIT_TIMEOUT,
171+
[userData]() -> bool { return userData->requestLifetimeShouldContinue; });
172+
}
173+
162174
if (meta_request_result->error_code != AWS_ERROR_SUCCESS && meta_request_result->response_status == 0) {
163175
/* client side error */
164176
userData->response->SetClientErrorType(CoreErrors::NETWORK_CONNECTION);

‎tools/code-generation/generator/src/main/resources/com/amazonaws/util/awsclientgenerator/velocity/cpp/s3/s3-crt/SmithyS3CrtSpecificOperations.vm‎

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -11,10 +11,16 @@ void S3CrtClient::CrtClientShutdownCallback(void *data)
1111
wrappedData->clientShutdownSem->Release();
1212
}
1313

14-
void S3CrtClient::CancelCrtRequestAsync(aws_s3_meta_request *meta_request) const {
14+
void S3CrtClient::CancelCrtRequestAsync(aws_s3_meta_request* meta_request, S3CrtClient::CrtRequestCallbackUserData* userData) const {
1515
assert(meta_request);
16-
m_clientConfiguration.executor->Submit([meta_request]() {
17-
aws_s3_meta_request_cancel(meta_request);
16+
userData->requestLifetimeShouldContinue = false;
17+
m_clientConfiguration.executor->Submit([meta_request, userData]() {
18+
{
19+
std::lock_guard<std::mutex> requestLifetimeLock{userData->requestLifetimeMutex};
20+
aws_s3_meta_request_cancel(meta_request);
21+
userData->requestLifetimeShouldContinue = true;
22+
}
23+
userData->requestLifetimeCondition.notify_all();
1824
});
1925
}
2026

@@ -38,7 +44,7 @@ int S3CrtClient::S3CrtRequestHeadersCallback(struct aws_s3_meta_request *meta_re
3844
auto& shouldContinueFn = userData->originalRequest->GetContinueRequestHandler();
3945
const HttpRequest* httpRequest = userData->request ? userData->request.get() : nullptr;
4046
if (shouldContinueFn && !shouldContinueFn(httpRequest)) {
41-
userData->s3CrtClient->CancelCrtRequestAsync(meta_request);
47+
userData->s3CrtClient->CancelCrtRequestAsync(meta_request, userData);
4248
}
4349

4450
return AWS_OP_SUCCESS;
@@ -73,7 +79,7 @@ int S3CrtClient::S3CrtRequestGetBodyCallback(struct aws_s3_meta_request *meta_re
7379
auto& shouldContinueFn = userData->originalRequest->GetContinueRequestHandler();
7480
const HttpRequest* httpRequest = userData->request ? userData->request.get() : nullptr;
7581
if (shouldContinueFn && !shouldContinueFn(httpRequest)) {
76-
userData->s3CrtClient->CancelCrtRequestAsync(meta_request);
82+
userData->s3CrtClient->CancelCrtRequestAsync(meta_request, userData);
7783
}
7884

7985
return AWS_OP_SUCCESS;
@@ -95,7 +101,7 @@ void S3CrtClient::S3CrtRequestProgressCallback(struct aws_s3_meta_request *meta_
95101
auto& shouldContinueFn = userData->originalRequest->GetContinueRequestHandler();
96102
const HttpRequest* httpRequest = userData->request ? userData->request.get() : nullptr;
97103
if (shouldContinueFn && !shouldContinueFn(httpRequest)) {
98-
userData->s3CrtClient->CancelCrtRequestAsync(meta_request);
104+
userData->s3CrtClient->CancelCrtRequestAsync(meta_request, userData);
99105
}
100106

101107
return;
@@ -159,6 +165,12 @@ void S3CrtClient::S3CrtRequestFinishCallback(struct aws_s3_meta_request *meta_re
159165
AWS_UNREFERENCED_PARAM(meta_request);
160166
auto *userData = static_cast<S3CrtClient::CrtRequestCallbackUserData*>(user_data);
161167

168+
{
169+
std::unique_lock<std::mutex> requestLifetimeLock{userData->requestLifetimeMutex};
170+
userData->requestLifetimeCondition.wait_for(requestLifetimeLock, REQUEST_LIFETIME_WAIT_TIMEOUT,
171+
[userData]() -> bool { return userData->requestLifetimeShouldContinue; });
172+
}
173+
162174
if (meta_request_result->error_code != AWS_ERROR_SUCCESS && meta_request_result->response_status == 0) {
163175
/* client side error */
164176
userData->response->SetClientErrorType(CoreErrors::NETWORK_CONNECTION);

0 commit comments

Comments
 (0)