Skip to content

Commit 3f57126

Browse files
bretambrosebretambrose
authored
Initial backwards-incompatible refactor of GG IPC to support safer object lifetimes (#377)
1 parent d99142e commit 3f57126

File tree

11 files changed

+198
-605
lines changed

11 files changed

+198
-605
lines changed

‎eventstream_rpc/include/aws/eventstreamrpc/EventStreamClient.h

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -492,12 +492,16 @@ namespace Aws
492492
public:
493493
ClientOperation(
494494
ClientConnection &connection,
495-
StreamResponseHandler *streamHandler,
495+
std::shared_ptr<StreamResponseHandler> streamHandler,
496496
const OperationModelContext &operationModelContext,
497497
Crt::Allocator *allocator) noexcept;
498498
~ClientOperation() noexcept;
499+
499500
ClientOperation(const ClientOperation &clientOperation) noexcept = delete;
500-
ClientOperation(ClientOperation &&clientOperation) noexcept;
501+
ClientOperation(ClientOperation &&clientOperation) noexcept = delete;
502+
bool operator=(const ClientOperation &clientOperation) noexcept = delete;
503+
bool operator=(ClientOperation &&clientOperation) noexcept = delete;
504+
501505
std::future<RpcError> Close(OnMessageFlushCallback onMessageFlushCallback = nullptr) noexcept;
502506
std::future<TaggedResult> GetOperationResult() noexcept;
503507

@@ -547,7 +551,7 @@ namespace Aws
547551

548552
uint32_t m_messageCount;
549553
Crt::Allocator *m_allocator;
550-
StreamResponseHandler *m_streamHandler;
554+
std::shared_ptr<StreamResponseHandler> m_streamHandler;
551555
ClientContinuation m_clientContinuation;
552556
/* This mutex protects m_resultReceived & m_closeState. */
553557
std::mutex m_continuationMutex;

‎eventstream_rpc/source/EventStreamClient.cpp

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1110,7 +1110,7 @@ namespace Aws
11101110

11111111
ClientOperation::ClientOperation(
11121112
ClientConnection &connection,
1113-
StreamResponseHandler *streamHandler,
1113+
std::shared_ptr<StreamResponseHandler> streamHandler,
11141114
const OperationModelContext &operationModelContext,
11151115
Crt::Allocator *allocator) noexcept
11161116
: m_operationModelContext(operationModelContext), m_messageCount(0), m_allocator(allocator),
@@ -1119,14 +1119,6 @@ namespace Aws
11191119
{
11201120
}
11211121

1122-
ClientOperation::ClientOperation(ClientOperation &&rhs) noexcept
1123-
: m_operationModelContext(rhs.m_operationModelContext), m_messageCount(std::move(rhs.m_messageCount)),
1124-
m_allocator(std::move(rhs.m_allocator)), m_streamHandler(rhs.m_streamHandler),
1125-
m_clientContinuation(std::move(rhs.m_clientContinuation)),
1126-
m_initialResponsePromise(std::move(rhs.m_initialResponsePromise))
1127-
{
1128-
}
1129-
11301122
ClientOperation::~ClientOperation() noexcept
11311123
{
11321124
Close().wait();

‎eventstream_rpc/tests/EchoTestRpcClient.cpp

Lines changed: 18 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -24,90 +24,50 @@ namespace Awstest
2424

2525
EchoTestRpcClient::~EchoTestRpcClient() noexcept { Close(); }
2626

27-
GetAllProductsOperation EchoTestRpcClient::NewGetAllProducts() noexcept
27+
std::shared_ptr<GetAllProductsOperation> EchoTestRpcClient::NewGetAllProducts() noexcept
2828
{
29-
return GetAllProductsOperation(
30-
m_connection, m_echoTestRpcServiceModel.m_getAllProductsOperationContext, m_allocator);
29+
return Aws::Crt::MakeShared<GetAllProductsOperation>(
30+
m_allocator, m_connection, m_echoTestRpcServiceModel.m_getAllProductsOperationContext, m_allocator);
3131
}
3232

33-
std::unique_ptr<GetAllProductsOperation> EchoTestRpcClient::NewPtrGetAllProducts() noexcept
33+
std::shared_ptr<CauseServiceErrorOperation> EchoTestRpcClient::NewCauseServiceError() noexcept
3434
{
35-
return std::unique_ptr<GetAllProductsOperation>(Aws::Crt::New<GetAllProductsOperation>(
36-
m_allocator, m_connection, m_echoTestRpcServiceModel.m_getAllProductsOperationContext, m_allocator));
35+
return Aws::Crt::MakeShared<CauseServiceErrorOperation>(
36+
m_allocator, m_connection, m_echoTestRpcServiceModel.m_causeServiceErrorOperationContext, m_allocator);
3737
}
3838

39-
CauseServiceErrorOperation EchoTestRpcClient::NewCauseServiceError() noexcept
40-
{
41-
return CauseServiceErrorOperation(
42-
m_connection, m_echoTestRpcServiceModel.m_causeServiceErrorOperationContext, m_allocator);
43-
}
44-
45-
std::unique_ptr<CauseServiceErrorOperation> EchoTestRpcClient::NewPtrCauseServiceError() noexcept
46-
{
47-
return std::unique_ptr<CauseServiceErrorOperation>(Aws::Crt::New<CauseServiceErrorOperation>(
48-
m_allocator, m_connection, m_echoTestRpcServiceModel.m_causeServiceErrorOperationContext, m_allocator));
49-
}
50-
51-
CauseStreamServiceToErrorOperation EchoTestRpcClient::NewCauseStreamServiceToError(
52-
CauseStreamServiceToErrorStreamHandler &streamHandler) noexcept
53-
{
54-
return CauseStreamServiceToErrorOperation(
55-
m_connection,
56-
&streamHandler,
57-
m_echoTestRpcServiceModel.m_causeStreamServiceToErrorOperationContext,
58-
m_allocator);
59-
}
60-
61-
std::unique_ptr<CauseStreamServiceToErrorOperation> EchoTestRpcClient::NewPtrCauseStreamServiceToError(
39+
std::shared_ptr<CauseStreamServiceToErrorOperation> EchoTestRpcClient::NewCauseStreamServiceToError(
6240
std::shared_ptr<CauseStreamServiceToErrorStreamHandler> streamHandler) noexcept
6341
{
64-
return std::unique_ptr<CauseStreamServiceToErrorOperation>(Aws::Crt::New<CauseStreamServiceToErrorOperation>(
42+
return Aws::Crt::MakeShared<CauseStreamServiceToErrorOperation>(
6543
m_allocator,
6644
m_connection,
6745
std::move(streamHandler),
6846
m_echoTestRpcServiceModel.m_causeStreamServiceToErrorOperationContext,
69-
m_allocator));
70-
}
71-
72-
EchoStreamMessagesOperation EchoTestRpcClient::NewEchoStreamMessages(
73-
EchoStreamMessagesStreamHandler &streamHandler) noexcept
74-
{
75-
return EchoStreamMessagesOperation(
76-
m_connection, &streamHandler, m_echoTestRpcServiceModel.m_echoStreamMessagesOperationContext, m_allocator);
47+
m_allocator);
7748
}
7849

79-
std::unique_ptr<EchoStreamMessagesOperation> EchoTestRpcClient::NewPtrEchoStreamMessages(
50+
std::shared_ptr<EchoStreamMessagesOperation> EchoTestRpcClient::NewEchoStreamMessages(
8051
std::shared_ptr<EchoStreamMessagesStreamHandler> streamHandler) noexcept
8152
{
82-
return std::unique_ptr<EchoStreamMessagesOperation>(Aws::Crt::New<EchoStreamMessagesOperation>(
53+
return Aws::Crt::MakeShared<EchoStreamMessagesOperation>(
8354
m_allocator,
8455
m_connection,
8556
std::move(streamHandler),
8657
m_echoTestRpcServiceModel.m_echoStreamMessagesOperationContext,
87-
m_allocator));
88-
}
89-
90-
EchoMessageOperation EchoTestRpcClient::NewEchoMessage() noexcept
91-
{
92-
return EchoMessageOperation(m_connection, m_echoTestRpcServiceModel.m_echoMessageOperationContext, m_allocator);
93-
}
94-
95-
std::unique_ptr<EchoMessageOperation> EchoTestRpcClient::NewPtrEchoMessage() noexcept
96-
{
97-
return std::unique_ptr<EchoMessageOperation>(Aws::Crt::New<EchoMessageOperation>(
98-
m_allocator, m_connection, m_echoTestRpcServiceModel.m_echoMessageOperationContext, m_allocator));
58+
m_allocator);
9959
}
10060

101-
GetAllCustomersOperation EchoTestRpcClient::NewGetAllCustomers() noexcept
61+
std::shared_ptr<EchoMessageOperation> EchoTestRpcClient::NewEchoMessage() noexcept
10262
{
103-
return GetAllCustomersOperation(
104-
m_connection, m_echoTestRpcServiceModel.m_getAllCustomersOperationContext, m_allocator);
63+
return Aws::Crt::MakeShared<EchoMessageOperation>(
64+
m_allocator, m_connection, m_echoTestRpcServiceModel.m_echoMessageOperationContext, m_allocator);
10565
}
10666

107-
std::unique_ptr<GetAllCustomersOperation> EchoTestRpcClient::NewPtrGetAllCustomers() noexcept
67+
std::shared_ptr<GetAllCustomersOperation> EchoTestRpcClient::NewGetAllCustomers() noexcept
10868
{
109-
return std::unique_ptr<GetAllCustomersOperation>(Aws::Crt::New<GetAllCustomersOperation>(
110-
m_allocator, m_connection, m_echoTestRpcServiceModel.m_getAllCustomersOperationContext, m_allocator));
69+
return Aws::Crt::MakeShared<GetAllCustomersOperation>(
70+
m_allocator, m_connection, m_echoTestRpcServiceModel.m_getAllCustomersOperationContext, m_allocator);
11171
}
11272

11373
} // namespace Awstest

‎eventstream_rpc/tests/EchoTestRpcModel.cpp

Lines changed: 2 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1141,22 +1141,12 @@ namespace Awstest
11411141
std::launch::deferred, [this]() { return CauseStreamServiceToErrorResult(GetOperationResult().get()); });
11421142
}
11431143

1144-
CauseStreamServiceToErrorOperation::CauseStreamServiceToErrorOperation(
1145-
ClientConnection &connection,
1146-
CauseStreamServiceToErrorStreamHandler *streamHandler,
1147-
const CauseStreamServiceToErrorOperationContext &operationContext,
1148-
Aws::Crt::Allocator *allocator) noexcept
1149-
: ClientOperation(connection, streamHandler, operationContext, allocator)
1150-
{
1151-
}
1152-
11531144
CauseStreamServiceToErrorOperation::CauseStreamServiceToErrorOperation(
11541145
ClientConnection &connection,
11551146
std::shared_ptr<CauseStreamServiceToErrorStreamHandler> streamHandler,
11561147
const CauseStreamServiceToErrorOperationContext &operationContext,
11571148
Aws::Crt::Allocator *allocator) noexcept
1158-
: ClientOperation(connection, streamHandler.get(), operationContext, allocator),
1159-
pinnedHandler(std::move(streamHandler))
1149+
: ClientOperation(connection, streamHandler, operationContext, allocator)
11601150
{
11611151
}
11621152

@@ -1238,22 +1228,12 @@ namespace Awstest
12381228
std::launch::deferred, [this]() { return EchoStreamMessagesResult(GetOperationResult().get()); });
12391229
}
12401230

1241-
EchoStreamMessagesOperation::EchoStreamMessagesOperation(
1242-
ClientConnection &connection,
1243-
EchoStreamMessagesStreamHandler *streamHandler,
1244-
const EchoStreamMessagesOperationContext &operationContext,
1245-
Aws::Crt::Allocator *allocator) noexcept
1246-
: ClientOperation(connection, streamHandler, operationContext, allocator)
1247-
{
1248-
}
1249-
12501231
EchoStreamMessagesOperation::EchoStreamMessagesOperation(
12511232
ClientConnection &connection,
12521233
std::shared_ptr<EchoStreamMessagesStreamHandler> streamHandler,
12531234
const EchoStreamMessagesOperationContext &operationContext,
12541235
Aws::Crt::Allocator *allocator) noexcept
1255-
: ClientOperation(connection, streamHandler.get(), operationContext, allocator),
1256-
pinnedHandler(std::move(streamHandler))
1236+
: ClientOperation(connection, streamHandler, operationContext, allocator)
12571237
{
12581238
}
12591239

‎eventstream_rpc/tests/EventStreamClientTest.cpp

Lines changed: 36 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -181,9 +181,9 @@ static int s_TestOperationWhileDisconnected(struct aws_allocator *allocator, voi
181181
Aws::Crt::String expectedMessage("l33t");
182182
messageData.SetStringMessage(expectedMessage);
183183
echoMessageRequest.SetMessage(messageData);
184-
auto requestFuture = echoMessage.Activate(echoMessageRequest, s_onMessageFlush);
184+
auto requestFuture = echoMessage->Activate(echoMessageRequest, s_onMessageFlush);
185185
ASSERT_TRUE(requestFuture.get().baseStatus == EVENT_STREAM_RPC_CONNECTION_CLOSED);
186-
auto result = echoMessage.GetOperationResult().get();
186+
auto result = echoMessage->GetOperationResult().get();
187187
ASSERT_FALSE(result);
188188
auto error = result.GetRpcError();
189189
ASSERT_TRUE(error.baseStatus == EVENT_STREAM_RPC_CONNECTION_CLOSED);
@@ -218,9 +218,9 @@ static int s_TestEchoOperation(struct aws_allocator *allocator, void *ctx)
218218
auto echoMessage = client.NewEchoMessage();
219219
messageData.SetStringMessage(expectedMessage);
220220
echoMessageRequest.SetMessage(messageData);
221-
auto requestFuture = echoMessage.Activate(echoMessageRequest, s_onMessageFlush);
221+
auto requestFuture = echoMessage->Activate(echoMessageRequest, s_onMessageFlush);
222222
requestFuture.wait();
223-
auto result = echoMessage.GetResult().get();
223+
auto result = echoMessage->GetResult().get();
224224
ASSERT_TRUE(result);
225225
auto response = result.GetOperationResponse();
226226
ASSERT_NOT_NULL(response);
@@ -240,9 +240,9 @@ static int s_TestEchoOperation(struct aws_allocator *allocator, void *ctx)
240240
Aws::Crt::String expectedMessage("l33t");
241241
messageData.SetStringMessage(expectedMessage);
242242
echoMessageRequest.SetMessage(messageData);
243-
auto requestFuture = echoMessage.Activate(echoMessageRequest, s_onMessageFlush);
243+
auto requestFuture = echoMessage->Activate(echoMessageRequest, s_onMessageFlush);
244244
ASSERT_TRUE(requestFuture.get().baseStatus == EVENT_STREAM_RPC_CONNECTION_CLOSED);
245-
auto result = echoMessage.GetOperationResult().get();
245+
auto result = echoMessage->GetOperationResult().get();
246246
ASSERT_FALSE(result);
247247
auto error = result.GetRpcError();
248248
ASSERT_TRUE(error.baseStatus == EVENT_STREAM_RPC_CONNECTION_CLOSED);
@@ -257,14 +257,14 @@ static int s_TestEchoOperation(struct aws_allocator *allocator, void *ctx)
257257
auto echoMessage = client.NewEchoMessage();
258258
messageData.SetStringMessage(expectedMessage);
259259
echoMessageRequest.SetMessage(messageData);
260-
auto requestFuture = echoMessage.Activate(echoMessageRequest, s_onMessageFlush);
261-
requestFuture = echoMessage.Activate(echoMessageRequest, s_onMessageFlush);
260+
auto requestFuture = echoMessage->Activate(echoMessageRequest, s_onMessageFlush);
261+
requestFuture = echoMessage->Activate(echoMessageRequest, s_onMessageFlush);
262262
MessageData differentMessage;
263263
differentMessage.SetBooleanMessage(true);
264264
echoMessageRequest.SetMessage(differentMessage);
265-
requestFuture = echoMessage.Activate(echoMessageRequest, s_onMessageFlush);
265+
requestFuture = echoMessage->Activate(echoMessageRequest, s_onMessageFlush);
266266
requestFuture.wait();
267-
auto result = echoMessage.GetResult().get();
267+
auto result = echoMessage->GetResult().get();
268268
ASSERT_TRUE(result);
269269
auto response = result.GetOperationResponse();
270270
ASSERT_NOT_NULL(response);
@@ -280,17 +280,17 @@ static int s_TestEchoOperation(struct aws_allocator *allocator, void *ctx)
280280
auto echoMessage = client.NewEchoMessage();
281281
messageData.SetStringMessage(expectedMessage);
282282
echoMessageRequest.SetMessage(messageData);
283-
auto requestFuture = echoMessage.Activate(echoMessageRequest, s_onMessageFlush);
284-
requestFuture = echoMessage.Activate(echoMessageRequest, s_onMessageFlush);
283+
auto requestFuture = echoMessage->Activate(echoMessageRequest, s_onMessageFlush);
284+
requestFuture = echoMessage->Activate(echoMessageRequest, s_onMessageFlush);
285285
MessageData differentMessage;
286286
differentMessage.SetBooleanMessage(true);
287287
echoMessageRequest.SetMessage(differentMessage);
288-
requestFuture = echoMessage.Activate(echoMessageRequest, s_onMessageFlush);
288+
requestFuture = echoMessage->Activate(echoMessageRequest, s_onMessageFlush);
289289
requestFuture.wait();
290-
echoMessage.Close().wait();
291-
echoMessage.Close().wait();
292-
echoMessage.Close().wait();
293-
echoMessage.Close().wait();
290+
echoMessage->Close().wait();
291+
echoMessage->Close().wait();
292+
echoMessage->Close().wait();
293+
echoMessage->Close().wait();
294294
}
295295

296296
/* Close without waiting on activation or close futures. */
@@ -302,16 +302,16 @@ static int s_TestEchoOperation(struct aws_allocator *allocator, void *ctx)
302302
auto echoMessage = client.NewEchoMessage();
303303
messageData.SetStringMessage(expectedMessage);
304304
echoMessageRequest.SetMessage(messageData);
305-
auto requestFuture = echoMessage.Activate(echoMessageRequest, s_onMessageFlush);
306-
requestFuture = echoMessage.Activate(echoMessageRequest, s_onMessageFlush);
305+
auto requestFuture = echoMessage->Activate(echoMessageRequest, s_onMessageFlush);
306+
requestFuture = echoMessage->Activate(echoMessageRequest, s_onMessageFlush);
307307
MessageData differentMessage;
308308
differentMessage.SetBooleanMessage(true);
309309
echoMessageRequest.SetMessage(differentMessage);
310-
echoMessage.Activate(echoMessageRequest, s_onMessageFlush);
311-
echoMessage.Close();
312-
echoMessage.Close();
313-
echoMessage.Close();
314-
echoMessage.Close();
310+
echoMessage->Activate(echoMessageRequest, s_onMessageFlush);
311+
echoMessage->Close();
312+
echoMessage->Close();
313+
echoMessage->Close();
314+
echoMessage->Close();
315315
}
316316

317317
/* Close without waiting for TERMINATE_STREAM to flush then immediately trying to activate. */
@@ -323,15 +323,15 @@ static int s_TestEchoOperation(struct aws_allocator *allocator, void *ctx)
323323
auto echoMessage = client.NewEchoMessage();
324324
messageData.SetStringMessage(expectedMessage);
325325
echoMessageRequest.SetMessage(messageData);
326-
auto requestFuture = echoMessage.Activate(echoMessageRequest, s_onMessageFlush);
327-
requestFuture = echoMessage.Activate(echoMessageRequest, s_onMessageFlush);
326+
auto requestFuture = echoMessage->Activate(echoMessageRequest, s_onMessageFlush);
327+
requestFuture = echoMessage->Activate(echoMessageRequest, s_onMessageFlush);
328328
MessageData differentMessage;
329329
differentMessage.SetBooleanMessage(true);
330330
echoMessageRequest.SetMessage(differentMessage);
331-
requestFuture = echoMessage.Activate(echoMessageRequest, s_onMessageFlush);
331+
requestFuture = echoMessage->Activate(echoMessageRequest, s_onMessageFlush);
332332
requestFuture.wait();
333-
auto closeFuture = echoMessage.Close();
334-
requestFuture = echoMessage.Activate(echoMessageRequest, s_onMessageFlush);
333+
auto closeFuture = echoMessage->Close();
334+
requestFuture = echoMessage->Activate(echoMessageRequest, s_onMessageFlush);
335335
closeFuture.wait();
336336
requestFuture.wait();
337337
}
@@ -349,9 +349,9 @@ static int s_TestEchoOperation(struct aws_allocator *allocator, void *ctx)
349349
ASSERT_TRUE(failedStatus.get().baseStatus == EVENT_STREAM_RPC_CONNECTION_ALREADY_ESTABLISHED);
350350
auto echoMessage = client.NewEchoMessage();
351351
echoMessageRequest.SetMessage(messageData);
352-
auto requestFuture = echoMessage.Activate(echoMessageRequest, s_onMessageFlush);
352+
auto requestFuture = echoMessage->Activate(echoMessageRequest, s_onMessageFlush);
353353
requestFuture.wait();
354-
auto result = echoMessage.GetResult().get();
354+
auto result = echoMessage->GetResult().get();
355355
ASSERT_TRUE(result);
356356
auto response = result.GetOperationResponse();
357357
ASSERT_NOT_NULL(response);
@@ -368,9 +368,9 @@ static int s_TestEchoOperation(struct aws_allocator *allocator, void *ctx)
368368
ASSERT_TRUE(connectedStatus.get().baseStatus == EVENT_STREAM_RPC_CONNECTION_ALREADY_ESTABLISHED);
369369
auto echoMessage = client.NewEchoMessage();
370370
echoMessageRequest.SetMessage(messageData);
371-
auto requestFuture = echoMessage.Activate(echoMessageRequest, s_onMessageFlush);
371+
auto requestFuture = echoMessage->Activate(echoMessageRequest, s_onMessageFlush);
372372
requestFuture.wait();
373-
auto result = echoMessage.GetResult().get();
373+
auto result = echoMessage->GetResult().get();
374374
ASSERT_TRUE(result);
375375
auto response = result.GetOperationResponse();
376376
ASSERT_NOT_NULL(response);
@@ -510,16 +510,16 @@ static int s_TestStressClient(struct aws_allocator *allocator, void *ctx)
510510
auto echoMessage = client.NewEchoMessage();
511511
messageData.SetStringMessage(expectedMessage);
512512
echoMessageRequest.SetMessage(messageData);
513-
auto requestStatus = echoMessage.Activate(echoMessageRequest, s_onMessageFlush).get();
514-
auto resultFuture = echoMessage.GetResult();
513+
auto requestStatus = echoMessage->Activate(echoMessageRequest, s_onMessageFlush).get();
514+
auto resultFuture = echoMessage->GetResult();
515515
/* The response may never arrive depending on how many ongoing requests are made
516516
* so in case of timeout, assume success. */
517517
std::future_status status = resultFuture.wait_for(std::chrono::seconds(5));
518518
if (status != std::future_status::ready)
519519
{
520520
return AWS_OP_SUCCESS;
521521
}
522-
auto result = echoMessage.GetResult().get();
522+
auto result = echoMessage->GetResult().get();
523523
ASSERT_TRUE(result);
524524
auto response = result.GetOperationResponse();
525525
ASSERT_NOT_NULL(response);

0 commit comments

Comments
 (0)