Skip to content

Commit b09cd08

Browse files
sbisciglsbiscigl
committed
make request cancel sync to avoid lifetime issues in CRT
1 parent b4ebf79 commit b09cd08

File tree

7 files changed

+18
-72
lines changed

7 files changed

+18
-72
lines changed

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

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8146,9 +8146,6 @@ 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;
81528149
};
81538150

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

81728169
static void CrtClientShutdownCallback(void* data);
8173-
void CancelCrtRequestAsync(aws_s3_meta_request* meta_request, S3CrtClient::CrtRequestCallbackUserData* userData) const;
8170+
void CancelCrtRequest(aws_s3_meta_request* meta_request) const;
81748171
static int S3CrtRequestHeadersCallback(aws_s3_meta_request* meta_request, const struct aws_http_headers* headers, int response_status,
81758172
void* user_data);
81768173
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: 5 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -165,8 +165,6 @@ 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-
170168
S3CrtClient::S3CrtClient(const S3CrtClient& rhs)
171169
: BASECLASS(rhs.m_clientConfiguration,
172170
Aws::MakeShared<Aws::Auth::S3ExpressSignerProvider>(
@@ -503,17 +501,9 @@ void S3CrtClient::CrtClientShutdownCallback(void* data) {
503501
wrappedData->clientShutdownSem->Release();
504502
}
505503

506-
void S3CrtClient::CancelCrtRequestAsync(aws_s3_meta_request* meta_request, S3CrtClient::CrtRequestCallbackUserData* userData) const {
504+
void S3CrtClient::CancelCrtRequest(aws_s3_meta_request* meta_request) const {
507505
assert(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-
});
506+
aws_s3_meta_request_cancel(meta_request);
517507
}
518508

519509
int S3CrtClient::S3CrtRequestHeadersCallback(struct aws_s3_meta_request* meta_request, const struct aws_http_headers* headers,
@@ -535,7 +525,7 @@ int S3CrtClient::S3CrtRequestHeadersCallback(struct aws_s3_meta_request* meta_re
535525
auto& shouldContinueFn = userData->originalRequest->GetContinueRequestHandler();
536526
const HttpRequest* httpRequest = userData->request ? userData->request.get() : nullptr;
537527
if (shouldContinueFn && !shouldContinueFn(httpRequest)) {
538-
userData->s3CrtClient->CancelCrtRequestAsync(meta_request, userData);
528+
userData->s3CrtClient->CancelCrtRequest(meta_request);
539529
}
540530

541531
return AWS_OP_SUCCESS;
@@ -568,7 +558,7 @@ int S3CrtClient::S3CrtRequestGetBodyCallback(struct aws_s3_meta_request* meta_re
568558
auto& shouldContinueFn = userData->originalRequest->GetContinueRequestHandler();
569559
const HttpRequest* httpRequest = userData->request ? userData->request.get() : nullptr;
570560
if (shouldContinueFn && !shouldContinueFn(httpRequest)) {
571-
userData->s3CrtClient->CancelCrtRequestAsync(meta_request, userData);
561+
userData->s3CrtClient->CancelCrtRequest(meta_request);
572562
}
573563

574564
return AWS_OP_SUCCESS;
@@ -590,7 +580,7 @@ void S3CrtClient::S3CrtRequestProgressCallback(struct aws_s3_meta_request* meta_
590580
auto& shouldContinueFn = userData->originalRequest->GetContinueRequestHandler();
591581
const HttpRequest* httpRequest = userData->request ? userData->request.get() : nullptr;
592582
if (shouldContinueFn && !shouldContinueFn(httpRequest)) {
593-
userData->s3CrtClient->CancelCrtRequestAsync(meta_request, userData);
583+
userData->s3CrtClient->CancelCrtRequest(meta_request);
594584
}
595585

596586
return;
@@ -653,12 +643,6 @@ void S3CrtClient::S3CrtRequestFinishCallback(struct aws_s3_meta_request* meta_re
653643
AWS_UNREFERENCED_PARAM(meta_request);
654644
auto* userData = static_cast<S3CrtClient::CrtRequestCallbackUserData*>(user_data);
655645

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-
662646
if (meta_request_result->error_code != AWS_ERROR_SUCCESS && meta_request_result->response_status == 0) {
663647
/* client side error */
664648
userData->response->SetClientErrorType(CoreErrors::NETWORK_CONNECTION);

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

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -207,9 +207,6 @@ 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;
213210
};
214211

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

235232
static void CrtClientShutdownCallback(void *data);
236-
void CancelCrtRequestAsync(aws_s3_meta_request *meta_request, S3CrtClient::CrtRequestCallbackUserData* userData) const;
233+
void CancelCrtRequest(aws_s3_meta_request *meta_request) const;
237234
static int S3CrtRequestHeadersCallback(aws_s3_meta_request *meta_request, const struct aws_http_headers *headers, int response_status, void *user_data);
238235
static int S3CrtRequestGetBodyCallback(struct aws_s3_meta_request *meta_request, const struct aws_byte_cursor *body, uint64_t range_start, void *user_data);
239236
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: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -231,9 +231,6 @@ 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;
237234
};
238235

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

255252
static void CrtClientShutdownCallback(void *data);
256-
void CancelCrtRequestAsync(aws_s3_meta_request *meta_request, S3CrtClient::CrtRequestCallbackUserData* userData) const;
253+
void CancelCrtRequest(aws_s3_meta_request *meta_request) const;
257254
static int S3CrtRequestHeadersCallback(aws_s3_meta_request *meta_request, const struct aws_http_headers *headers, int response_status, void *user_data);
258255
static int S3CrtRequestGetBodyCallback(struct aws_s3_meta_request *meta_request, const struct aws_byte_cursor *body, uint64_t range_start, void *user_data);
259256
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: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,6 @@
5555
#set($hasEventStreamInputOperation = true)
5656
#end
5757
#end
58-
const std::chrono::minutes REQUEST_LIFETIME_WAIT_TIMEOUT = std::chrono::minutes(1);
5958

6059
${className}::${className}(const ${className} &rhs) :
6160
BASECLASS(rhs.m_clientConfiguration,

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

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

14-
void S3CrtClient::CancelCrtRequestAsync(aws_s3_meta_request *meta_request, S3CrtClient::CrtRequestCallbackUserData* userData) const {
14+
void S3CrtClient::CancelCrtRequest(aws_s3_meta_request *meta_request) const {
1515
assert(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();
24-
});
16+
aws_s3_meta_request_cancel(meta_request);
2517
}
2618

2719
int S3CrtClient::S3CrtRequestHeadersCallback(struct aws_s3_meta_request *meta_request, const struct aws_http_headers *headers,
@@ -44,7 +36,7 @@ int S3CrtClient::S3CrtRequestHeadersCallback(struct aws_s3_meta_request *meta_re
4436
auto& shouldContinueFn = userData->originalRequest->GetContinueRequestHandler();
4537
const HttpRequest* httpRequest = userData->request ? userData->request.get() : nullptr;
4638
if (shouldContinueFn && !shouldContinueFn(httpRequest)) {
47-
userData->s3CrtClient->CancelCrtRequestAsync(meta_request, userData);
39+
userData->s3CrtClient->CancelCrtRequest(meta_request);
4840
}
4941

5042
return AWS_OP_SUCCESS;
@@ -79,7 +71,7 @@ int S3CrtClient::S3CrtRequestGetBodyCallback(struct aws_s3_meta_request *meta_re
7971
auto& shouldContinueFn = userData->originalRequest->GetContinueRequestHandler();
8072
const HttpRequest* httpRequest = userData->request ? userData->request.get() : nullptr;
8173
if (shouldContinueFn && !shouldContinueFn(httpRequest)) {
82-
userData->s3CrtClient->CancelCrtRequestAsync(meta_request, userData);
74+
userData->s3CrtClient->CancelCrtRequest(meta_request);
8375
}
8476

8577
return AWS_OP_SUCCESS;
@@ -101,7 +93,7 @@ void S3CrtClient::S3CrtRequestProgressCallback(struct aws_s3_meta_request *meta_
10193
auto& shouldContinueFn = userData->originalRequest->GetContinueRequestHandler();
10294
const HttpRequest* httpRequest = userData->request ? userData->request.get() : nullptr;
10395
if (shouldContinueFn && !shouldContinueFn(httpRequest)) {
104-
userData->s3CrtClient->CancelCrtRequestAsync(meta_request, userData);
96+
userData->s3CrtClient->CancelCrtRequest(meta_request);
10597
}
10698

10799
return;
@@ -165,12 +157,6 @@ void S3CrtClient::S3CrtRequestFinishCallback(struct aws_s3_meta_request *meta_re
165157
AWS_UNREFERENCED_PARAM(meta_request);
166158
auto *userData = static_cast<S3CrtClient::CrtRequestCallbackUserData*>(user_data);
167159

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-
174160
if (meta_request_result->error_code != AWS_ERROR_SUCCESS && meta_request_result->response_status == 0) {
175161
/* client side error */
176162
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: 5 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -11,17 +11,9 @@ void S3CrtClient::CrtClientShutdownCallback(void *data)
1111
wrappedData->clientShutdownSem->Release();
1212
}
1313

14-
void S3CrtClient::CancelCrtRequestAsync(aws_s3_meta_request* meta_request, S3CrtClient::CrtRequestCallbackUserData* userData) const {
14+
void S3CrtClient::CancelCrtRequest(aws_s3_meta_request* meta_request) const {
1515
assert(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();
24-
});
16+
aws_s3_meta_request_cancel(meta_request);
2517
}
2618

2719
int S3CrtClient::S3CrtRequestHeadersCallback(struct aws_s3_meta_request *meta_request, const struct aws_http_headers *headers,
@@ -44,7 +36,7 @@ int S3CrtClient::S3CrtRequestHeadersCallback(struct aws_s3_meta_request *meta_re
4436
auto& shouldContinueFn = userData->originalRequest->GetContinueRequestHandler();
4537
const HttpRequest* httpRequest = userData->request ? userData->request.get() : nullptr;
4638
if (shouldContinueFn && !shouldContinueFn(httpRequest)) {
47-
userData->s3CrtClient->CancelCrtRequestAsync(meta_request, userData);
39+
userData->s3CrtClient->CancelCrtRequest(meta_request);
4840
}
4941

5042
return AWS_OP_SUCCESS;
@@ -79,7 +71,7 @@ int S3CrtClient::S3CrtRequestGetBodyCallback(struct aws_s3_meta_request *meta_re
7971
auto& shouldContinueFn = userData->originalRequest->GetContinueRequestHandler();
8072
const HttpRequest* httpRequest = userData->request ? userData->request.get() : nullptr;
8173
if (shouldContinueFn && !shouldContinueFn(httpRequest)) {
82-
userData->s3CrtClient->CancelCrtRequestAsync(meta_request, userData);
74+
userData->s3CrtClient->CancelCrtRequest(meta_request);
8375
}
8476

8577
return AWS_OP_SUCCESS;
@@ -101,7 +93,7 @@ void S3CrtClient::S3CrtRequestProgressCallback(struct aws_s3_meta_request *meta_
10193
auto& shouldContinueFn = userData->originalRequest->GetContinueRequestHandler();
10294
const HttpRequest* httpRequest = userData->request ? userData->request.get() : nullptr;
10395
if (shouldContinueFn && !shouldContinueFn(httpRequest)) {
104-
userData->s3CrtClient->CancelCrtRequestAsync(meta_request, userData);
96+
userData->s3CrtClient->CancelCrtRequest(meta_request);
10597
}
10698

10799
return;
@@ -165,12 +157,6 @@ void S3CrtClient::S3CrtRequestFinishCallback(struct aws_s3_meta_request *meta_re
165157
AWS_UNREFERENCED_PARAM(meta_request);
166158
auto *userData = static_cast<S3CrtClient::CrtRequestCallbackUserData*>(user_data);
167159

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-
174160
if (meta_request_result->error_code != AWS_ERROR_SUCCESS && meta_request_result->response_status == 0) {
175161
/* client side error */
176162
userData->response->SetClientErrorType(CoreErrors::NETWORK_CONNECTION);

0 commit comments

Comments
 (0)