Request Response Duplicate Correlation Token Validation

[sbSteveK]

If a Request-Response Operation attempts to use an already in-use correlation token it will now fail validation on submission of the operation to the RR Client.

Add a test for failing validation when a duplicate correlation token is submitted while the token is already in use.
Add a test for successful validation of a reused correlation token if the previous operation with the same correlation token has already completed.

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

[codecov-commenter]

Codecov Report

Attention: Patch coverage is 96.00000% with 1 line in your changes missing coverage. Please review.

Project coverage is 84.71%. Comparing base (0024652) to head (f6d1761).

Files with missing lines	Patch %	Lines
source/request-response/request_response_client.c	96.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #400      +/-   ##
==========================================
+ Coverage   84.70%   84.71%   +0.01%     
==========================================
  Files          25       25              
  Lines       10492    10517      +25     
==========================================
+ Hits         8887     8910      +23     
- Misses       1605     1607       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[sfod]

I think s_remove_correlation_token_from_in_use should be called only once in s_remove_operation_from_client_tables. I.e. an operation's token should become available for reuse only when there is no possibility for the operation to do something with it.

With current implementation, it's hard to reason about if it's possible or not for the previous operation to do something with its token when the token is already assigned to the next operation.

For example, take the following scenario.

Imagine there is an existing operation op_1 with a token token.

• a new operation op_2 with a token token started by a user thread, so aws_mqtt_request_response_client_submit_request executes:
  • schedule a task (for the next event loop iteration) which will perform this new operation initialization

event loop iteration N:

• a PUBLISH arrives for op_1, so s_complete_operation_with_correlation_token executes:
  • rm token from correlation_tokens_in_use
  • ... perform the rest of the function
• Initialize op_2, i.e. execute s_mqtt_rr_client_submit_operation function:
  • add token to correlation_tokens_in_use
  • ... and do other stuff

event loop iteration N + 1:

• the second PUBLISH arrives for op_1 (or alternatively there was an op failure on the previous iteration)
  • rm token from correlation_tokens_in_use

Now there is op_2 with a token token, but no token record in correlation_tokens_in_use.

To address this hypothetical scenario, we could just execute s_remove_correlation_token_from_in_use(operation); only if state not AWS_MRROS_PENDING_DESTROY. But I feel that this will be still kinda fragile.

[sbSteveK]

I think the reason I went with the way I did was that we need to have a separate check in s_complete_request_operation_with_failure() to insure we aren't removing a correlation token when the operation is cleaning up due to a duplicate correlation token error. I'll trace the logic and see if we can't combine it and work through your scenario.