From 909277d164532f09543d7bf39aedd97d1601b585 Mon Sep 17 00:00:00 2001 From: MushMal <33669149+MushMal@users.noreply.github.com> Date: Tue, 14 Apr 2020 14:50:34 -0700 Subject: [PATCH] Adding delete signaling channel API (#346) * Adding delete signaling channel API Adding tests and fixing some of the existing tests * Adding TSAN suppression for the new test path --- .../kinesis/video/webrtcclient/Include.h | 48 +++- src/source/Signaling/Client.c | 24 ++ src/source/Signaling/LwsApiCalls.c | 17 +- src/source/Signaling/Signaling.c | 71 +++++- src/source/Signaling/Signaling.h | 13 + src/source/Signaling/StateMachine.c | 182 ++++++++++++- src/source/Signaling/StateMachine.h | 6 + tst/IceFunctionalityTest.cpp | 4 + tst/SignalingApiFunctionalityTest.cpp | 239 ++++++++++++++++++ tst/SignalingApiTest.cpp | 64 +++-- tst/WebRTCClientTestFixture.cpp | 36 +-- tst/WebRTCClientTestFixture.h | 1 + tst/suppressions/TSAN.supp | 18 ++ 13 files changed, 644 insertions(+), 79 deletions(-) diff --git a/src/include/com/amazonaws/kinesis/video/webrtcclient/Include.h b/src/include/com/amazonaws/kinesis/video/webrtcclient/Include.h index 976b41c8d9..b5bc54404c 100644 --- a/src/include/com/amazonaws/kinesis/video/webrtcclient/Include.h +++ b/src/include/com/amazonaws/kinesis/video/webrtcclient/Include.h @@ -268,6 +268,7 @@ extern "C" { #define STATUS_SIGNALING_INVALID_MESSAGE_TTL_VALUE STATUS_SIGNALING_BASE + 0x0000002E #define STATUS_SIGNALING_ICE_CONFIG_REFRESH_FAILED STATUS_SIGNALING_BASE + 0x0000002F #define STATUS_SIGNALING_RECONNECT_FAILED STATUS_SIGNALING_BASE + 0x00000030 +#define STATUS_SIGNALING_DELETE_CALL_FAILED STATUS_SIGNALING_BASE + 0x00000031 /*!@} */ /*===========================================================================================*/ @@ -532,6 +533,11 @@ extern "C" { */ #define SIGNALING_SEND_TIMEOUT (5 * HUNDREDS_OF_NANOS_IN_A_SECOND) +/** + * Default timeout for deleting a channel + */ +#define SIGNALING_DELETE_TIMEOUT (5 * HUNDREDS_OF_NANOS_IN_A_SECOND) + /** * Default signaling message alive time */ @@ -782,6 +788,8 @@ typedef enum { //!< we get to this state after ICE refresh SIGNALING_CLIENT_STATE_CONNECTED, //!< On transitioning to this state, the timeout on the state machine is reset SIGNALING_CLIENT_STATE_DISCONNECTED, //!< This state transition happens either from connect or connected state + SIGNALING_CLIENT_STATE_DELETE, //!< This state transition happens when the application calls signalingClientDeleteSync API. + SIGNALING_CLIENT_STATE_DELETED, //!< This state transition happens after the channel gets deleted as a result of a signalingClientDeleteSync API. This is a terminal state. SIGNALING_CLIENT_STATE_MAX_VALUE, //!< This state indicates maximum number of signaling client states } SIGNALING_CLIENT_STATE, *PSIGNALING_CLIENT_STATE; @@ -1557,7 +1565,7 @@ PUBLIC_API STATUS signalingClientGetIceConfigInfoCount(SIGNALING_CLIENT_HANDLE, * IMPORTANT: The returned pointer to the ICE configuration information object points to internal structures * and its contents should not be modified. * - * @param SIGNALING_CLIENT_HANDLE + * @param[in] SIGNALING_CLIENT_HANDLE Signaling client handle * @param[in] UINT32 Index of the ICE configuration information object to retrieve * @param[out] PIceConfigInfo The pointer to the ICE configuration information object * @@ -1565,37 +1573,55 @@ PUBLIC_API STATUS signalingClientGetIceConfigInfoCount(SIGNALING_CLIENT_HANDLE, */ PUBLIC_API STATUS signalingClientGetIceConfigInfo(SIGNALING_CLIENT_HANDLE, UINT32, PIceConfigInfo*); - /** * @brief Connects the signaling client to the socket in order to send/receive messages. * * NOTE: The call will succeed only when the signaling client is in a ready state. - * @param SIGNALING_CLIENT_HANDLE * - * @return STATUS function execution status. STATUS_SUCCESS on success + * @param[in] SIGNALING_CLIENT_HANDLE Signaling client handle + * + * @return STATUS code of the execution. STATUS_SUCCESS on success */ PUBLIC_API STATUS signalingClientConnectSync(SIGNALING_CLIENT_HANDLE); /* - * Gets the Signaling client current state. + * @brief Gets the Signaling client current state. * - * @param - SIGNALING_CLIENT_HANDLE - IN - Signaling client handle - * @param - PSIGNALING_CLIENT_STATE - OUT - Current state of the signaling client as an UINT32 enum + * @param[in] SIGNALING_CLIENT_HANDLE Signaling client handle + * @param[out] PSIGNALING_CLIENT_STATE Current state of the signaling client as an UINT32 enum * - * @return - STATUS code of the execution + * @return STATUS code of the execution. STATUS_SUCCESS on success */ PUBLIC_API STATUS signalingClientGetCurrentState(SIGNALING_CLIENT_HANDLE, PSIGNALING_CLIENT_STATE); /* * Gets a literal string representing a Signaling client state. * - * @param - SIGNALING_CLIENT_STATE - IN - Signaling client state - * @param - PCHAR* - OUT - Read only string representing the state + * @param[in] SIGNALING_CLIENT_HANDLE Signaling client handle + * @param[out] PCHAR* Read only string representing the state * - * @return - STATUS code of the execution + * @return STATUS code of the execution. STATUS_SUCCESS on success */ PUBLIC_API STATUS signalingClientGetStateString(SIGNALING_CLIENT_STATE, PCHAR*); +/** + * @brief Deletes the signaling channel referenced by SIGNALING_CLIENT_HANDLE + * + * NOTE: The function is intended to be used to clean up the backend resources and + * as such should be called at the end of the lifecycle of the signaling channel resource. + * Attempting to connect to the channel or send a message will result in an + * error or an unpredictable results after this call. + * + * NOTE: The call transitions the signaling client state machine to a terminal state + * even if the call fails. The proper handling on success and on an error is to + * free the signaling client. + * + * @param[in] SIGNALING_CLIENT_HANDLE Signaling client handle + * + * @return STATUS code of the execution. STATUS_SUCCESS on success + */ +PUBLIC_API STATUS signalingClientDeleteSync(SIGNALING_CLIENT_HANDLE); + #ifdef __cplusplus } #endif diff --git a/src/source/Signaling/Client.c b/src/source/Signaling/Client.c index f787c38552..747c84243d 100644 --- a/src/source/Signaling/Client.c +++ b/src/source/Signaling/Client.c @@ -88,6 +88,22 @@ STATUS signalingClientConnectSync(SIGNALING_CLIENT_HANDLE signalingClientHandle) return retStatus; } +STATUS signalingClientDeleteSync(SIGNALING_CLIENT_HANDLE signalingClientHandle) +{ + ENTERS(); + STATUS retStatus = STATUS_SUCCESS; + PSignalingClient pSignalingClient = FROM_SIGNALING_CLIENT_HANDLE(signalingClientHandle); + + DLOGI("Signaling Client Delete Sync"); + + CHK_STATUS(signalingDeleteSync(pSignalingClient)); + +CleanUp: + + LEAVES(); + return retStatus; +} + STATUS signalingClientGetIceConfigInfoCount(SIGNALING_CLIENT_HANDLE signalingClientHandle, PUINT32 pIceConfigCount) { ENTERS(); @@ -193,6 +209,14 @@ STATUS signalingClientGetStateString(SIGNALING_CLIENT_STATE state, PCHAR* ppStat *ppStateStr = SIGNALING_CLIENT_STATE_DISCONNECTED_STR; break; + case SIGNALING_CLIENT_STATE_DELETE: + *ppStateStr = SIGNALING_CLIENT_STATE_DELETE_STR; + break; + + case SIGNALING_CLIENT_STATE_DELETED: + *ppStateStr = SIGNALING_CLIENT_STATE_DELETED_STR; + break; + case SIGNALING_CLIENT_STATE_MAX_VALUE: case SIGNALING_CLIENT_STATE_UNKNOWN: // Explicit fall-through diff --git a/src/source/Signaling/LwsApiCalls.c b/src/source/Signaling/LwsApiCalls.c index ca1edab04a..0f4656b44a 100644 --- a/src/source/Signaling/LwsApiCalls.c +++ b/src/source/Signaling/LwsApiCalls.c @@ -1121,6 +1121,7 @@ STATUS deleteChannelLws(PSignalingClient pSignalingClient, UINT64 time) CHAR url[MAX_URI_CHAR_LEN + 1]; CHAR paramsJson[MAX_JSON_PARAMETER_STRING_LEN]; PLwsCallInfo pLwsCallInfo = NULL; + SIZE_T result; CHK(pSignalingClient != NULL, STATUS_NULL_ARG); CHK(pSignalingClient->channelDescription.channelArn[0] != '\0', STATUS_INVALID_OPERATION); @@ -1167,8 +1168,13 @@ STATUS deleteChannelLws(PSignalingClient pSignalingClient, UINT64 time) // Set the service call result ATOMIC_STORE(&pSignalingClient->result, (SIZE_T) pLwsCallInfo->callInfo.callResult); - // Early return if we have a non-success result - CHK((SERVICE_CALL_RESULT) ATOMIC_LOAD(&pSignalingClient->result) == SERVICE_CALL_RESULT_OK, retStatus); + // Early return if we have a non-success result and it's not a resource not found + result = ATOMIC_LOAD(&pSignalingClient->result); + CHK((SERVICE_CALL_RESULT) result == SERVICE_CALL_RESULT_OK || + (SERVICE_CALL_RESULT) result == SERVICE_CALL_RESOURCE_NOT_FOUND, retStatus); + + // Mark the channel as deleted + ATOMIC_STORE_BOOL(&pSignalingClient->deleted, TRUE); CleanUp: @@ -1879,13 +1885,6 @@ STATUS terminateLwsListenerLoop(PSignalingClient pSignalingClient) terminateConnectionWithStatus(pSignalingClient, SERVICE_CALL_RESULT_OK); } - if (pSignalingClient->pLwsContext != NULL) { - MUTEX_LOCK(pSignalingClient->lwsSerializerLock); - lws_context_destroy(pSignalingClient->pLwsContext); - pSignalingClient->pLwsContext = NULL; - MUTEX_UNLOCK(pSignalingClient->lwsSerializerLock); - } - CleanUp: LEAVES(); diff --git a/src/source/Signaling/Signaling.c b/src/source/Signaling/Signaling.c index e4d0de0971..bd49c149a3 100644 --- a/src/source/Signaling/Signaling.c +++ b/src/source/Signaling/Signaling.c @@ -97,6 +97,8 @@ STATUS createSignalingSync(PSignalingClientInfoInternal pClientInfo, PChannelInf ATOMIC_STORE_BOOL(&pSignalingClient->clientReady, FALSE); ATOMIC_STORE_BOOL(&pSignalingClient->shutdown, FALSE); ATOMIC_STORE_BOOL(&pSignalingClient->connected, FALSE); + ATOMIC_STORE_BOOL(&pSignalingClient->deleting, FALSE); + ATOMIC_STORE_BOOL(&pSignalingClient->deleted, FALSE); // Add to the signal handler // signal(SIGINT, lwsSignalHandler); @@ -179,13 +181,14 @@ STATUS freeSignaling(PSignalingClient* ppSignalingClient) ATOMIC_STORE_BOOL(&pSignalingClient->shutdown, TRUE); - timerQueueFree(&pSignalingClient->timerQueueHandle); - - // Terminate the listener thread if alive - terminateLwsListenerLoop(pSignalingClient); + terminateOngoingOperations(pSignalingClient); - // Await for the reconnect thread to exit - awaitForThreadTermination(&pSignalingClient->reconnecterTracker, SIGNALING_CLIENT_SHUTDOWN_TIMEOUT); + if (pSignalingClient->pLwsContext != NULL) { + MUTEX_LOCK(pSignalingClient->lwsSerializerLock); + lws_context_destroy(pSignalingClient->pLwsContext); + pSignalingClient->pLwsContext = NULL; + MUTEX_UNLOCK(pSignalingClient->lwsSerializerLock); + } freeStateMachine(pSignalingClient->pStateMachine); @@ -246,11 +249,34 @@ STATUS freeSignaling(PSignalingClient* ppSignalingClient) return retStatus; } +STATUS terminateOngoingOperations(PSignalingClient pSignalingClient) +{ + ENTERS(); + STATUS retStatus = STATUS_SUCCESS; + + CHK(pSignalingClient != NULL, STATUS_NULL_ARG); + + timerQueueFree(&pSignalingClient->timerQueueHandle); + + // Terminate the listener thread if alive + terminateLwsListenerLoop(pSignalingClient); + + // Await for the reconnect thread to exit + awaitForThreadTermination(&pSignalingClient->reconnecterTracker, SIGNALING_CLIENT_SHUTDOWN_TIMEOUT); + +CleanUp: + + CHK_LOG_ERR(retStatus); + + LEAVES(); + return retStatus; +} + STATUS signalingSendMessageSync(PSignalingClient pSignalingClient, PSignalingMessage pSignalingMessage) { ENTERS(); STATUS retStatus = STATUS_SUCCESS; - PCHAR pOfferType; + PCHAR pOfferType = NULL; BOOL removeFromList = FALSE; CHK(pSignalingClient != NULL && pSignalingMessage != NULL, STATUS_NULL_ARG); @@ -374,6 +400,37 @@ STATUS signalingConnectSync(PSignalingClient pSignalingClient) return retStatus; } +STATUS signalingDeleteSync(PSignalingClient pSignalingClient) +{ + ENTERS(); + STATUS retStatus = STATUS_SUCCESS; + + CHK(pSignalingClient != NULL, STATUS_NULL_ARG); + + // Check if we are already deleting + CHK (!ATOMIC_LOAD_BOOL(&pSignalingClient->deleted), retStatus); + + // Mark as being deleted + ATOMIC_STORE_BOOL(&pSignalingClient->deleting, TRUE); + + CHK_STATUS(terminateOngoingOperations(pSignalingClient)); + + // Set the state directly + setStateMachineCurrentState(pSignalingClient->pStateMachine, SIGNALING_STATE_DELETE); + + // Set the time out before execution + pSignalingClient->stepUntil = GETTIME() + SIGNALING_DELETE_TIMEOUT; + + CHK_STATUS(stepSignalingStateMachine(pSignalingClient, retStatus)); + +CleanUp: + + CHK_LOG_ERR(retStatus); + + LEAVES(); + return retStatus; +} + STATUS validateSignalingCallbacks(PSignalingClient pSignalingClient, PSignalingClientCallbacks pCallbacks) { ENTERS(); diff --git a/src/source/Signaling/Signaling.h b/src/source/Signaling/Signaling.h index 5869da6161..708e112bda 100644 --- a/src/source/Signaling/Signaling.h +++ b/src/source/Signaling/Signaling.h @@ -35,6 +35,8 @@ extern "C" { #define SIGNALING_CLIENT_STATE_CONNECTING_STR "Connecting" #define SIGNALING_CLIENT_STATE_CONNECTED_STR "Connected" #define SIGNALING_CLIENT_STATE_DISCONNECTED_STR "Disconnected" +#define SIGNALING_CLIENT_STATE_DELETE_STR "Delete" +#define SIGNALING_CLIENT_STATE_DELETED_STR "Deleted" // Error refreshing ICE server configuration string #define SIGNALING_ICE_CONFIG_REFRESH_ERROR_MSG "Failed refreshing ICE server configuration with status code 0x%08x." @@ -82,6 +84,8 @@ typedef struct { SignalingApiCallHookFunc getIceConfigPostHookFn; SignalingApiCallHookFunc connectPreHookFn; SignalingApiCallHookFunc connectPostHookFn; + SignalingApiCallHookFunc deletePreHookFn; + SignalingApiCallHookFunc deletePostHookFn; } SignalingClientInfoInternal, *PSignalingClientInfoInternal; /** @@ -113,6 +117,12 @@ typedef struct { // Wss is connected volatile ATOMIC_BOOL connected; + // The channel is being deleted + volatile ATOMIC_BOOL deleting; + + // The channel is deleted + volatile ATOMIC_BOOL deleted; + // Current version of the structure UINT32 version; @@ -221,6 +231,7 @@ STATUS signalingSendMessageSync(PSignalingClient, PSignalingMessage); STATUS signalingGetIceConfigInfoCout(PSignalingClient, PUINT32); STATUS signalingGetIceConfigInfo(PSignalingClient, UINT32, PIceConfigInfo*); STATUS signalingConnectSync(PSignalingClient); +STATUS signalingDeleteSync(PSignalingClient); STATUS validateSignalingCallbacks(PSignalingClient, PSignalingClientCallbacks); STATUS validateSignalingClientInfo(PSignalingClient, PSignalingClientInfoInternal); @@ -236,6 +247,8 @@ STATUS awaitForThreadTermination(PThreadTracker, UINT64); STATUS initializeThreadTracker(PThreadTracker); STATUS uninitializeThreadTracker(PThreadTracker); +STATUS terminateOngoingOperations(PSignalingClient); + #ifdef __cplusplus } #endif diff --git a/src/source/Signaling/StateMachine.c b/src/source/Signaling/StateMachine.c index ade1dca179..2835a27ee3 100644 --- a/src/source/Signaling/StateMachine.c +++ b/src/source/Signaling/StateMachine.c @@ -9,8 +9,8 @@ */ StateMachineState SIGNALING_STATE_MACHINE_STATES[] = { {SIGNALING_STATE_NEW, SIGNALING_STATE_NONE | SIGNALING_STATE_NEW, fromNewSignalingState, executeNewSignalingState, INFINITE_RETRY_COUNT_SENTINEL, STATUS_SIGNALING_INVALID_READY_STATE}, - {SIGNALING_STATE_GET_TOKEN, SIGNALING_STATE_NEW | SIGNALING_STATE_DESCRIBE | SIGNALING_STATE_CREATE | SIGNALING_STATE_GET_ENDPOINT | SIGNALING_STATE_GET_ICE_CONFIG | SIGNALING_STATE_READY | SIGNALING_STATE_CONNECT | SIGNALING_STATE_CONNECTED | SIGNALING_STATE_GET_TOKEN, fromGetTokenSignalingState, executeGetTokenSignalingState, SIGNALING_STATES_DEFAULT_RETRY_COUNT, STATUS_SIGNALING_GET_TOKEN_CALL_FAILED}, - {SIGNALING_STATE_DESCRIBE, SIGNALING_STATE_GET_TOKEN | SIGNALING_STATE_CREATE | SIGNALING_STATE_GET_ENDPOINT | SIGNALING_STATE_GET_ICE_CONFIG | SIGNALING_STATE_CONNECT | SIGNALING_STATE_CONNECTED | SIGNALING_STATE_DESCRIBE, fromDescribeSignalingState, executeDescribeSignalingState, SIGNALING_STATES_DEFAULT_RETRY_COUNT, STATUS_SIGNALING_DESCRIBE_CALL_FAILED}, + {SIGNALING_STATE_GET_TOKEN, SIGNALING_STATE_NEW | SIGNALING_STATE_DESCRIBE | SIGNALING_STATE_CREATE | SIGNALING_STATE_GET_ENDPOINT | SIGNALING_STATE_GET_ICE_CONFIG | SIGNALING_STATE_READY | SIGNALING_STATE_CONNECT | SIGNALING_STATE_CONNECTED | SIGNALING_STATE_DELETE | SIGNALING_STATE_GET_TOKEN, fromGetTokenSignalingState, executeGetTokenSignalingState, SIGNALING_STATES_DEFAULT_RETRY_COUNT, STATUS_SIGNALING_GET_TOKEN_CALL_FAILED}, + {SIGNALING_STATE_DESCRIBE, SIGNALING_STATE_GET_TOKEN | SIGNALING_STATE_CREATE | SIGNALING_STATE_GET_ENDPOINT | SIGNALING_STATE_GET_ICE_CONFIG | SIGNALING_STATE_CONNECT | SIGNALING_STATE_CONNECTED | SIGNALING_STATE_DELETE | SIGNALING_STATE_DESCRIBE, fromDescribeSignalingState, executeDescribeSignalingState, SIGNALING_STATES_DEFAULT_RETRY_COUNT, STATUS_SIGNALING_DESCRIBE_CALL_FAILED}, {SIGNALING_STATE_CREATE, SIGNALING_STATE_DESCRIBE | SIGNALING_STATE_CREATE, fromCreateSignalingState, executeCreateSignalingState, SIGNALING_STATES_DEFAULT_RETRY_COUNT, STATUS_SIGNALING_CREATE_CALL_FAILED}, {SIGNALING_STATE_GET_ENDPOINT, SIGNALING_STATE_DESCRIBE | SIGNALING_STATE_CREATE | SIGNALING_STATE_GET_TOKEN | SIGNALING_STATE_READY | SIGNALING_STATE_CONNECT | SIGNALING_STATE_CONNECTED | SIGNALING_STATE_GET_ENDPOINT, fromGetEndpointSignalingState, executeGetEndpointSignalingState, SIGNALING_STATES_DEFAULT_RETRY_COUNT, STATUS_SIGNALING_GET_ENDPOINT_CALL_FAILED}, {SIGNALING_STATE_GET_ICE_CONFIG, SIGNALING_STATE_DESCRIBE | SIGNALING_STATE_CONNECTED | SIGNALING_STATE_GET_ENDPOINT | SIGNALING_STATE_READY | SIGNALING_STATE_GET_ICE_CONFIG, fromGetIceConfigSignalingState, executeGetIceConfigSignalingState, SIGNALING_STATES_DEFAULT_RETRY_COUNT, STATUS_SIGNALING_GET_ICE_CONFIG_CALL_FAILED}, @@ -18,6 +18,8 @@ StateMachineState SIGNALING_STATE_MACHINE_STATES[] = { {SIGNALING_STATE_CONNECT, SIGNALING_STATE_READY | SIGNALING_STATE_DISCONNECTED | SIGNALING_STATE_CONNECT, fromConnectSignalingState, executeConnectSignalingState, INFINITE_RETRY_COUNT_SENTINEL, STATUS_SIGNALING_CONNECT_CALL_FAILED}, {SIGNALING_STATE_CONNECTED, SIGNALING_STATE_CONNECT | SIGNALING_STATE_CONNECTED, fromConnectedSignalingState, executeConnectedSignalingState, INFINITE_RETRY_COUNT_SENTINEL, STATUS_SIGNALING_CONNECTED_CALLBACK_FAILED}, {SIGNALING_STATE_DISCONNECTED, SIGNALING_STATE_CONNECT | SIGNALING_STATE_CONNECTED, fromDisconnectedSignalingState, executeDisconnectedSignalingState, SIGNALING_STATES_DEFAULT_RETRY_COUNT, STATUS_SIGNALING_DISCONNECTED_CALLBACK_FAILED}, + {SIGNALING_STATE_DELETE, SIGNALING_STATE_GET_TOKEN | SIGNALING_STATE_DESCRIBE | SIGNALING_STATE_CREATE | SIGNALING_STATE_GET_ENDPOINT | SIGNALING_STATE_GET_ICE_CONFIG | SIGNALING_STATE_READY | SIGNALING_STATE_CONNECT | SIGNALING_STATE_CONNECTED | SIGNALING_STATE_DISCONNECTED | SIGNALING_STATE_DELETE, fromDeleteSignalingState, executeDeleteSignalingState, SIGNALING_STATES_DEFAULT_RETRY_COUNT, STATUS_SIGNALING_DELETE_CALL_FAILED}, + {SIGNALING_STATE_DELETED, SIGNALING_STATE_DELETE | SIGNALING_STATE_DELETED, fromDeletedSignalingState, executeDeletedSignalingState, INFINITE_RETRY_COUNT_SENTINEL, STATUS_SIGNALING_DELETE_CALL_FAILED}, }; UINT32 SIGNALING_STATE_MACHINE_STATE_COUNT = ARRAY_SIZE(SIGNALING_STATE_MACHINE_STATES); @@ -114,6 +116,12 @@ SIGNALING_CLIENT_STATE getSignalingStateFromStateMachineState(UINT64 state) case SIGNALING_STATE_DISCONNECTED: clientState = SIGNALING_CLIENT_STATE_DISCONNECTED; break; + case SIGNALING_STATE_DELETE: + clientState = SIGNALING_CLIENT_STATE_DELETE; + break; + case SIGNALING_STATE_DELETED: + clientState = SIGNALING_CLIENT_STATE_DELETED; + break; default: clientState = SIGNALING_CLIENT_STATE_UNKNOWN; } @@ -201,10 +209,15 @@ STATUS fromGetTokenSignalingState(UINT64 customData, PUINT64 pState) CHK(pSignalingClient != NULL && pState != NULL, STATUS_NULL_ARG); if ((SERVICE_CALL_RESULT) ATOMIC_LOAD(&pSignalingClient->result) == SERVICE_CALL_RESULT_OK) { - // If the client application has specified the Channel ARN then we will skip describe and create states - if (pSignalingClient->pChannelInfo->pChannelArn != NULL && pSignalingClient->pChannelInfo->pChannelArn[0] != '\0') { + // Check if we are trying to delete a channel + if (ATOMIC_LOAD_BOOL(&pSignalingClient->deleting)) { + state = SIGNALING_STATE_DELETE; + } else if (pSignalingClient->pChannelInfo->pChannelArn != NULL && + pSignalingClient->pChannelInfo->pChannelArn[0] != '\0') { + // If the client application has specified the Channel ARN then we will skip describe and create states // Store the ARN in the stream description object first - STRNCPY(pSignalingClient->channelDescription.channelArn, pSignalingClient->pChannelInfo->pChannelArn, MAX_ARN_LEN); + STRNCPY(pSignalingClient->channelDescription.channelArn, pSignalingClient->pChannelInfo->pChannelArn, + MAX_ARN_LEN); pSignalingClient->channelDescription.channelArn[MAX_ARN_LEN] = '\0'; // Move to get endpoint state @@ -280,7 +293,13 @@ STATUS fromDescribeSignalingState(UINT64 customData, PUINT64 pState) result = ATOMIC_LOAD(&pSignalingClient->result); switch (result) { case SERVICE_CALL_RESULT_OK: - state = SIGNALING_STATE_GET_ENDPOINT; + // If we are trying to delete the channel then move to delete state + if (ATOMIC_LOAD_BOOL(&pSignalingClient->deleting)) { + state = SIGNALING_STATE_DELETE; + } else { + state = SIGNALING_STATE_GET_ENDPOINT; + } + break; case SERVICE_CALL_RESOURCE_NOT_FOUND: @@ -581,11 +600,27 @@ STATUS fromReadySignalingState(UINT64 customData, PUINT64 pState) PSignalingClient pSignalingClient = SIGNALING_CLIENT_FROM_CUSTOM_DATA(customData); UINT64 state = SIGNALING_STATE_CONNECT; + SIZE_T result; + CHK(pSignalingClient != NULL && pState != NULL, STATUS_NULL_ARG); - // Move to connect only when we had previously connected - if(SERVICE_CALL_RESULT_SIGNALING_RECONNECT_ICE == (SERVICE_CALL_RESULT) ATOMIC_LOAD(&pSignalingClient->result)) { - state = SIGNALING_STATE_GET_ICE_CONFIG; + result = ATOMIC_LOAD(&pSignalingClient->result); + switch (result) { + case SERVICE_CALL_RESULT_OK: + state = SIGNALING_STATE_READY; + break; + + case SERVICE_CALL_RESULT_SIGNALING_RECONNECT_ICE: + state = SIGNALING_STATE_GET_ICE_CONFIG; + break; + + case SERVICE_CALL_FORBIDDEN: + case SERVICE_CALL_NOT_AUTHORIZED: + state = SIGNALING_STATE_GET_TOKEN; + break; + + default: + break; } *pState = state; @@ -877,3 +912,132 @@ STATUS executeDisconnectedSignalingState(UINT64 customData, UINT64 time) LEAVES(); return retStatus; } + +STATUS fromDeleteSignalingState(UINT64 customData, PUINT64 pState) +{ + ENTERS(); + STATUS retStatus = STATUS_SUCCESS; + PSignalingClient pSignalingClient = SIGNALING_CLIENT_FROM_CUSTOM_DATA(customData); + UINT64 state = SIGNALING_STATE_DELETE; + + SIZE_T result; + + CHK(pSignalingClient != NULL && pState != NULL, STATUS_NULL_ARG); + + result = ATOMIC_LOAD(&pSignalingClient->result); + switch (result) { + case SERVICE_CALL_FORBIDDEN: + case SERVICE_CALL_NOT_AUTHORIZED: + state = SIGNALING_STATE_GET_TOKEN; + break; + + case SERVICE_CALL_RESULT_OK: + case SERVICE_CALL_RESOURCE_DELETED: + case SERVICE_CALL_RESOURCE_NOT_FOUND: + state = SIGNALING_STATE_DELETED; + break; + + case SERVICE_CALL_BAD_REQUEST: + // This can happen if we come in from specifying ARN and skipping Describe state + // during the creation in which case we still need to get the proper update version + state = SIGNALING_STATE_DESCRIBE; + break; + + default: + break; + } + + *pState = state; + +CleanUp: + + LEAVES(); + return retStatus; +} + +STATUS executeDeleteSignalingState(UINT64 customData, UINT64 time) +{ + ENTERS(); + STATUS retStatus = STATUS_SUCCESS; + PSignalingClient pSignalingClient = SIGNALING_CLIENT_FROM_CUSTOM_DATA(customData); + + CHK(pSignalingClient != NULL, STATUS_NULL_ARG); + ATOMIC_STORE(&pSignalingClient->result, (SIZE_T) SERVICE_CALL_RESULT_NOT_SET); + ATOMIC_STORE_BOOL(&pSignalingClient->clientReady, FALSE); + + // Notify of the state change + if (pSignalingClient->signalingClientCallbacks.stateChangeFn != NULL) { + CHK_STATUS(pSignalingClient->signalingClientCallbacks.stateChangeFn( + pSignalingClient->signalingClientCallbacks.customData, + SIGNALING_CLIENT_STATE_DELETE)); + } + + // Call pre hook func + if (pSignalingClient->clientInfo.deletePreHookFn != NULL) { + retStatus = pSignalingClient->clientInfo.deletePreHookFn(SIGNALING_STATE_DELETE, + pSignalingClient->clientInfo.hookCustomData); + } + + // Call DescribeChannel API + if (STATUS_SUCCEEDED(retStatus)) { + retStatus = deleteChannelLws(pSignalingClient, time); + } + + // Call post hook func + if (pSignalingClient->clientInfo.deletePostHookFn != NULL) { + retStatus = pSignalingClient->clientInfo.deletePostHookFn(SIGNALING_STATE_DELETE, + pSignalingClient->clientInfo.hookCustomData); + } + + CHK_STATUS(stepSignalingStateMachine(pSignalingClient, retStatus)); + + // Reset the ret status + retStatus = STATUS_SUCCESS; + +CleanUp: + + LEAVES(); + return retStatus; +} + +STATUS fromDeletedSignalingState(UINT64 customData, PUINT64 pState) +{ + ENTERS(); + STATUS retStatus = STATUS_SUCCESS; + PSignalingClient pSignalingClient = SIGNALING_CLIENT_FROM_CUSTOM_DATA(customData); + UINT64 state = SIGNALING_STATE_DELETED; + + CHK(pSignalingClient != NULL && pState != NULL, STATUS_NULL_ARG); + + // This is a terminal state + *pState = state; + +CleanUp: + + LEAVES(); + return retStatus; +} + +STATUS executeDeletedSignalingState(UINT64 customData, UINT64 time) +{ + ENTERS(); + UNUSED_PARAM(time); + STATUS retStatus = STATUS_SUCCESS; + PSignalingClient pSignalingClient = SIGNALING_CLIENT_FROM_CUSTOM_DATA(customData); + + CHK(pSignalingClient != NULL, STATUS_NULL_ARG); + + // Notify of the state change + if (pSignalingClient->signalingClientCallbacks.stateChangeFn != NULL) { + CHK_STATUS(pSignalingClient->signalingClientCallbacks.stateChangeFn( + pSignalingClient->signalingClientCallbacks.customData, + SIGNALING_CLIENT_STATE_DELETED)); + } + + // No-op + +CleanUp: + + LEAVES(); + return retStatus; +} diff --git a/src/source/Signaling/StateMachine.h b/src/source/Signaling/StateMachine.h index e0438b49af..7ba412b04c 100644 --- a/src/source/Signaling/StateMachine.h +++ b/src/source/Signaling/StateMachine.h @@ -24,6 +24,8 @@ extern "C" { #define SIGNALING_STATE_CONNECT ((UINT64) (1 << 7)) #define SIGNALING_STATE_CONNECTED ((UINT64) (1 << 8)) #define SIGNALING_STATE_DISCONNECTED ((UINT64) (1 << 9)) +#define SIGNALING_STATE_DELETE ((UINT64) (1 << 10)) +#define SIGNALING_STATE_DELETED ((UINT64) (1 << 11)) // Indicates infinite retries #define INFINITE_RETRY_COUNT_SENTINEL 0 @@ -57,6 +59,10 @@ STATUS fromConnectedSignalingState(UINT64, PUINT64); STATUS executeConnectedSignalingState(UINT64, UINT64); STATUS fromDisconnectedSignalingState(UINT64, PUINT64); STATUS executeDisconnectedSignalingState(UINT64, UINT64); +STATUS fromDeleteSignalingState(UINT64, PUINT64); +STATUS executeDeleteSignalingState(UINT64, UINT64); +STATUS fromDeletedSignalingState(UINT64, PUINT64); +STATUS executeDeletedSignalingState(UINT64, UINT64); #ifdef __cplusplus } diff --git a/tst/IceFunctionalityTest.cpp b/tst/IceFunctionalityTest.cpp index 6753573945..c34b6e8f19 100644 --- a/tst/IceFunctionalityTest.cpp +++ b/tst/IceFunctionalityTest.cpp @@ -534,6 +534,10 @@ namespace com { namespace amazonaws { namespace kinesis { namespace video { name TEST_F(IceFunctionalityTest, IceAgentCandidateGatheringTest) { + if (!mAccessKeyIdSet) { + return; + } + typedef struct { std::vector list; std::mutex lock; diff --git a/tst/SignalingApiFunctionalityTest.cpp b/tst/SignalingApiFunctionalityTest.cpp index ab006faa69..539a67a661 100644 --- a/tst/SignalingApiFunctionalityTest.cpp +++ b/tst/SignalingApiFunctionalityTest.cpp @@ -1708,6 +1708,245 @@ TEST_F(SignalingApiFunctionalityTest, channelInfoArnSkipDescribe) EXPECT_EQ(STATUS_SUCCESS, freeSignalingClient(&signalingHandle)); } +TEST_F(SignalingApiFunctionalityTest, deleteChannelCreatedWithArn) +{ + if (!mAccessKeyIdSet) { + return; + } + + ChannelInfo channelInfo; + SignalingClientCallbacks signalingClientCallbacks; + SignalingClientInfoInternal clientInfoInternal; + PSignalingClient pSignalingClient; + SIGNALING_CLIENT_HANDLE signalingHandle; + CHAR testArn[MAX_ARN_LEN + 1]; + + signalingClientCallbacks.version = SIGNALING_CLIENT_CALLBACKS_CURRENT_VERSION; + signalingClientCallbacks.customData = (UINT64) this; + signalingClientCallbacks.messageReceivedFn = NULL; + signalingClientCallbacks.errorReportFn = signalingClientError; + signalingClientCallbacks.stateChangeFn = signalingClientStateChanged; + + MEMSET(&clientInfoInternal, 0x00, SIZEOF(SignalingClientInfoInternal)); + + clientInfoInternal.signalingClientInfo.version = SIGNALING_CLIENT_INFO_CURRENT_VERSION; + clientInfoInternal.signalingClientInfo.loggingLevel = LOG_LEVEL_VERBOSE; + STRCPY(clientInfoInternal.signalingClientInfo.clientId, TEST_SIGNALING_MASTER_CLIENT_ID); + clientInfoInternal.iceRefreshPeriod = 0; + clientInfoInternal.connectTimeout = 0; + + MEMSET(&channelInfo, 0x00, SIZEOF(ChannelInfo)); + channelInfo.version = CHANNEL_INFO_CURRENT_VERSION; + channelInfo.pChannelName = mChannelName; + channelInfo.pKmsKeyId = NULL; + channelInfo.tagCount = 0; + channelInfo.pTags = NULL; + channelInfo.channelType = SIGNALING_CHANNEL_TYPE_SINGLE_MASTER; + channelInfo.channelRoleType = SIGNALING_CHANNEL_ROLE_TYPE_MASTER; + channelInfo.cachingEndpoint = FALSE; + channelInfo.retry = TRUE; + channelInfo.reconnect = TRUE; + channelInfo.pCertPath = mCaCertPath; + channelInfo.messageTtl = TEST_SIGNALING_MESSAGE_TTL; + + EXPECT_EQ(STATUS_SUCCESS, createSignalingSync(&clientInfoInternal, &channelInfo, &signalingClientCallbacks, + (PAwsCredentialProvider) mTestCredentialProvider, &pSignalingClient)); + signalingHandle = TO_SIGNALING_CLIENT_HANDLE(pSignalingClient); + EXPECT_TRUE(IS_VALID_SIGNALING_CLIENT_HANDLE(signalingHandle)); + + pActiveClient = pSignalingClient; + + // Check the states first + EXPECT_EQ(1, signalingStatesCounts[SIGNALING_CLIENT_STATE_NEW]); + EXPECT_EQ(1, signalingStatesCounts[SIGNALING_CLIENT_STATE_GET_CREDENTIALS]); + EXPECT_EQ(2, signalingStatesCounts[SIGNALING_CLIENT_STATE_DESCRIBE]); + EXPECT_EQ(1, signalingStatesCounts[SIGNALING_CLIENT_STATE_CREATE]); + EXPECT_EQ(1, signalingStatesCounts[SIGNALING_CLIENT_STATE_GET_ENDPOINT]); + EXPECT_EQ(1, signalingStatesCounts[SIGNALING_CLIENT_STATE_GET_ICE_CONFIG]); + EXPECT_EQ(1, signalingStatesCounts[SIGNALING_CLIENT_STATE_READY]); + EXPECT_EQ(0, signalingStatesCounts[SIGNALING_CLIENT_STATE_CONNECTING]); + EXPECT_EQ(0, signalingStatesCounts[SIGNALING_CLIENT_STATE_CONNECTED]); + EXPECT_EQ(0, signalingStatesCounts[SIGNALING_CLIENT_STATE_DISCONNECTED]); + + // Connect to the signaling client - should connect OK + EXPECT_EQ(STATUS_SUCCESS, signalingClientConnectSync(signalingHandle)); + + EXPECT_EQ(1, signalingStatesCounts[SIGNALING_CLIENT_STATE_CONNECTED]); + + // Check that we are connected and can send a message + SignalingMessage signalingMessage; + signalingMessage.version = SIGNALING_MESSAGE_CURRENT_VERSION; + signalingMessage.messageType = SIGNALING_MESSAGE_TYPE_OFFER; + STRCPY(signalingMessage.peerClientId, TEST_SIGNALING_MASTER_CLIENT_ID); + MEMSET(signalingMessage.payload, 'A', 100); + signalingMessage.payload[100] = '\0'; + signalingMessage.payloadLen = 0; + signalingMessage.correlationId[0] = '\0'; + + EXPECT_EQ(STATUS_SUCCESS, signalingClientSendMessageSync(signalingHandle, &signalingMessage)); + + // + // Store the ARN, free the client, repeat the same with the ARN, ensure we don't hit the describe and create states + // + STRCPY(testArn, pSignalingClient->channelDescription.channelArn); + + // Free the client, reset the states count + EXPECT_EQ(STATUS_SUCCESS, freeSignalingClient(&signalingHandle)); + MEMSET(signalingStatesCounts, 0x00, SIZEOF(signalingStatesCounts)); + + DLOGD("Attempting to create a signaling client for an existing channel %s with channel ARN %s", channelInfo.pChannelName, testArn); + + // Create channel with ARN and without name + channelInfo.pChannelName = NULL; + channelInfo.pChannelArn = testArn; + + EXPECT_EQ(STATUS_SUCCESS, createSignalingSync(&clientInfoInternal, &channelInfo, &signalingClientCallbacks, + (PAwsCredentialProvider) mTestCredentialProvider, &pSignalingClient)); + signalingHandle = TO_SIGNALING_CLIENT_HANDLE(pSignalingClient); + EXPECT_TRUE(IS_VALID_SIGNALING_CLIENT_HANDLE(signalingHandle)); + + pActiveClient = pSignalingClient; + + // Check the states first + EXPECT_EQ(1, signalingStatesCounts[SIGNALING_CLIENT_STATE_NEW]); + EXPECT_EQ(1, signalingStatesCounts[SIGNALING_CLIENT_STATE_GET_CREDENTIALS]); + EXPECT_EQ(0, signalingStatesCounts[SIGNALING_CLIENT_STATE_DESCRIBE]); + EXPECT_EQ(0, signalingStatesCounts[SIGNALING_CLIENT_STATE_CREATE]); + EXPECT_EQ(1, signalingStatesCounts[SIGNALING_CLIENT_STATE_GET_ENDPOINT]); + EXPECT_EQ(1, signalingStatesCounts[SIGNALING_CLIENT_STATE_GET_ICE_CONFIG]); + EXPECT_EQ(1, signalingStatesCounts[SIGNALING_CLIENT_STATE_READY]); + EXPECT_EQ(0, signalingStatesCounts[SIGNALING_CLIENT_STATE_CONNECTING]); + EXPECT_EQ(0, signalingStatesCounts[SIGNALING_CLIENT_STATE_CONNECTED]); + EXPECT_EQ(0, signalingStatesCounts[SIGNALING_CLIENT_STATE_DISCONNECTED]); + + // Connect to the signaling client - should connect OK + EXPECT_EQ(STATUS_SUCCESS, signalingClientConnectSync(signalingHandle)); + + EXPECT_EQ(1, signalingStatesCounts[SIGNALING_CLIENT_STATE_CONNECTED]); + + // Check that we are connected and can send a message + MEMSET(signalingMessage.payload, 'B', 50); + signalingMessage.payload[50] = '\0'; + signalingMessage.payloadLen = 0; + signalingMessage.correlationId[0] = '\0'; + + EXPECT_EQ(STATUS_SUCCESS, signalingClientSendMessageSync(signalingHandle, &signalingMessage)); + + EXPECT_EQ(STATUS_SUCCESS, signalingClientDeleteSync(signalingHandle)); + + EXPECT_EQ(STATUS_SUCCESS, freeSignalingClient(&signalingHandle)); +} + +TEST_F(SignalingApiFunctionalityTest, deleteChannelCreatedAuthExpiration) +{ + if (!mAccessKeyIdSet) { + return; + } + + ChannelInfo channelInfo; + SignalingClientCallbacks signalingClientCallbacks; + SignalingClientInfoInternal clientInfoInternal; + PSignalingClient pSignalingClient; + SIGNALING_CLIENT_HANDLE signalingHandle; + + signalingClientCallbacks.version = SIGNALING_CLIENT_CALLBACKS_CURRENT_VERSION; + signalingClientCallbacks.customData = (UINT64) this; + signalingClientCallbacks.messageReceivedFn = NULL; + signalingClientCallbacks.errorReportFn = signalingClientError; + signalingClientCallbacks.stateChangeFn = signalingClientStateChanged; + + MEMSET(&clientInfoInternal, 0x00, SIZEOF(SignalingClientInfoInternal)); + + clientInfoInternal.signalingClientInfo.version = SIGNALING_CLIENT_INFO_CURRENT_VERSION; + clientInfoInternal.signalingClientInfo.loggingLevel = LOG_LEVEL_VERBOSE; + STRCPY(clientInfoInternal.signalingClientInfo.clientId, TEST_SIGNALING_MASTER_CLIENT_ID); + clientInfoInternal.iceRefreshPeriod = 0; + clientInfoInternal.connectTimeout = 0; + + MEMSET(&channelInfo, 0x00, SIZEOF(ChannelInfo)); + channelInfo.version = CHANNEL_INFO_CURRENT_VERSION; + channelInfo.pChannelName = mChannelName; + channelInfo.pKmsKeyId = NULL; + channelInfo.tagCount = 0; + channelInfo.pTags = NULL; + channelInfo.channelType = SIGNALING_CHANNEL_TYPE_SINGLE_MASTER; + channelInfo.channelRoleType = SIGNALING_CHANNEL_ROLE_TYPE_MASTER; + channelInfo.cachingEndpoint = FALSE; + channelInfo.retry = TRUE; + channelInfo.reconnect = TRUE; + channelInfo.pCertPath = mCaCertPath; + channelInfo.messageTtl = TEST_SIGNALING_MESSAGE_TTL; + + // Force auth token refresh right after the main API calls + ((PStaticCredentialProvider) mTestCredentialProvider)->pAwsCredentials->expiration = GETTIME() + 4 * HUNDREDS_OF_NANOS_IN_A_SECOND; + + EXPECT_EQ(STATUS_SUCCESS, createSignalingSync(&clientInfoInternal, &channelInfo, &signalingClientCallbacks, + (PAwsCredentialProvider) mTestCredentialProvider, &pSignalingClient)); + signalingHandle = TO_SIGNALING_CLIENT_HANDLE(pSignalingClient); + EXPECT_TRUE(IS_VALID_SIGNALING_CLIENT_HANDLE(signalingHandle)); + + pActiveClient = pSignalingClient; + + // Check the states first + EXPECT_EQ(1, signalingStatesCounts[SIGNALING_CLIENT_STATE_NEW]); + EXPECT_EQ(1, signalingStatesCounts[SIGNALING_CLIENT_STATE_GET_CREDENTIALS]); + EXPECT_EQ(2, signalingStatesCounts[SIGNALING_CLIENT_STATE_DESCRIBE]); + EXPECT_EQ(1, signalingStatesCounts[SIGNALING_CLIENT_STATE_CREATE]); + EXPECT_EQ(1, signalingStatesCounts[SIGNALING_CLIENT_STATE_GET_ENDPOINT]); + EXPECT_EQ(1, signalingStatesCounts[SIGNALING_CLIENT_STATE_GET_ICE_CONFIG]); + EXPECT_EQ(1, signalingStatesCounts[SIGNALING_CLIENT_STATE_READY]); + EXPECT_EQ(0, signalingStatesCounts[SIGNALING_CLIENT_STATE_CONNECTING]); + EXPECT_EQ(0, signalingStatesCounts[SIGNALING_CLIENT_STATE_CONNECTED]); + EXPECT_EQ(0, signalingStatesCounts[SIGNALING_CLIENT_STATE_DISCONNECTED]); + EXPECT_EQ(0, signalingStatesCounts[SIGNALING_CLIENT_STATE_DELETE]); + EXPECT_EQ(0, signalingStatesCounts[SIGNALING_CLIENT_STATE_DELETED]); + + // Force the auth error on the delete API call + THREAD_SLEEP(7 * HUNDREDS_OF_NANOS_IN_A_SECOND); + EXPECT_NE(STATUS_SUCCESS, signalingClientDeleteSync(signalingHandle)); + + // Check the states - we should have failed on get credentials + EXPECT_EQ(1, signalingStatesCounts[SIGNALING_CLIENT_STATE_NEW]); + EXPECT_EQ(12, signalingStatesCounts[SIGNALING_CLIENT_STATE_GET_CREDENTIALS]); + EXPECT_EQ(2, signalingStatesCounts[SIGNALING_CLIENT_STATE_DESCRIBE]); + EXPECT_EQ(1, signalingStatesCounts[SIGNALING_CLIENT_STATE_CREATE]); + EXPECT_EQ(1, signalingStatesCounts[SIGNALING_CLIENT_STATE_GET_ENDPOINT]); + EXPECT_EQ(1, signalingStatesCounts[SIGNALING_CLIENT_STATE_GET_ICE_CONFIG]); + EXPECT_EQ(1, signalingStatesCounts[SIGNALING_CLIENT_STATE_READY]); + EXPECT_EQ(0, signalingStatesCounts[SIGNALING_CLIENT_STATE_CONNECTING]); + EXPECT_EQ(0, signalingStatesCounts[SIGNALING_CLIENT_STATE_CONNECTED]); + EXPECT_EQ(0, signalingStatesCounts[SIGNALING_CLIENT_STATE_DISCONNECTED]); + EXPECT_EQ(1, signalingStatesCounts[SIGNALING_CLIENT_STATE_DELETE]); + EXPECT_EQ(0, signalingStatesCounts[SIGNALING_CLIENT_STATE_DELETED]); + + // Reset it back right after the GetIce is called already + ((PStaticCredentialProvider) mTestCredentialProvider)->pAwsCredentials->expiration = MAX_UINT64; + + // Should succeed properly + EXPECT_EQ(STATUS_SUCCESS, signalingClientDeleteSync(signalingHandle)); + + // Check the states - we should have got the credentials now and directly moved to delete + EXPECT_EQ(1, signalingStatesCounts[SIGNALING_CLIENT_STATE_NEW]); + EXPECT_EQ(13, signalingStatesCounts[SIGNALING_CLIENT_STATE_GET_CREDENTIALS]); + EXPECT_EQ(2, signalingStatesCounts[SIGNALING_CLIENT_STATE_DESCRIBE]); + EXPECT_EQ(1, signalingStatesCounts[SIGNALING_CLIENT_STATE_CREATE]); + EXPECT_EQ(1, signalingStatesCounts[SIGNALING_CLIENT_STATE_GET_ENDPOINT]); + EXPECT_EQ(1, signalingStatesCounts[SIGNALING_CLIENT_STATE_GET_ICE_CONFIG]); + EXPECT_EQ(1, signalingStatesCounts[SIGNALING_CLIENT_STATE_READY]); + EXPECT_EQ(0, signalingStatesCounts[SIGNALING_CLIENT_STATE_CONNECTING]); + EXPECT_EQ(0, signalingStatesCounts[SIGNALING_CLIENT_STATE_CONNECTED]); + EXPECT_EQ(0, signalingStatesCounts[SIGNALING_CLIENT_STATE_DISCONNECTED]); + EXPECT_EQ(2, signalingStatesCounts[SIGNALING_CLIENT_STATE_DELETE]); + EXPECT_EQ(1, signalingStatesCounts[SIGNALING_CLIENT_STATE_DELETED]); + + // Shouldn't be able to connect as it's not in ready state + EXPECT_NE(STATUS_SUCCESS, signalingClientConnectSync(signalingHandle)); + + deleteChannelLws(FROM_SIGNALING_CLIENT_HANDLE(signalingHandle), 0); + + EXPECT_EQ(STATUS_SUCCESS, freeSignalingClient(&signalingHandle)); +} + } } } diff --git a/tst/SignalingApiTest.cpp b/tst/SignalingApiTest.cpp index 927f35e215..8aaae18f9e 100644 --- a/tst/SignalingApiTest.cpp +++ b/tst/SignalingApiTest.cpp @@ -7,10 +7,6 @@ class SignalingApiTest : public WebRtcClientTestBase { TEST_F(SignalingApiTest, signalingSendMessageSync) { - if (!mAccessKeyIdSet) { - return; - } - STATUS expectedStatus; SignalingMessage signalingMessage; @@ -54,11 +50,7 @@ TEST_F(SignalingApiTest, signalingSendMessageSync) TEST_F(SignalingApiTest, signalingClientConnectSync) { - if (!mAccessKeyIdSet) { - return; - } - - STATUS expectedStatus; + STATUS expectedStatus; initializeSignalingClient(); EXPECT_NE(STATUS_SUCCESS, signalingClientConnectSync(INVALID_SIGNALING_CLIENT_HANDLE_VALUE)); @@ -66,19 +58,44 @@ TEST_F(SignalingApiTest, signalingClientConnectSync) EXPECT_EQ(expectedStatus, signalingClientConnectSync(mSignalingClientHandle)); // Connect again + EXPECT_EQ(expectedStatus, signalingClientConnectSync(mSignalingClientHandle)); + EXPECT_EQ(expectedStatus, signalingClientConnectSync(mSignalingClientHandle)); + + deinitializeSignalingClient(); +} + +TEST_F(SignalingApiTest, signalingClientDeleteSync) +{ + STATUS expectedStatus; + + initializeSignalingClient(); + EXPECT_NE(STATUS_SUCCESS, signalingClientDeleteSync(INVALID_SIGNALING_CLIENT_HANDLE_VALUE)); + expectedStatus = mAccessKeyIdSet ? STATUS_SUCCESS : STATUS_NULL_ARG; + EXPECT_EQ(expectedStatus, signalingClientDeleteSync(mSignalingClientHandle)); + + // Call again - idempotent + EXPECT_EQ(expectedStatus, signalingClientDeleteSync(mSignalingClientHandle)); + + // Attempt to call a connect should fail expectedStatus = mAccessKeyIdSet ? STATUS_INVALID_STREAM_STATE : STATUS_NULL_ARG; - EXPECT_EQ(STATUS_SUCCESS, signalingClientConnectSync(mSignalingClientHandle)); - EXPECT_EQ(STATUS_SUCCESS, signalingClientConnectSync(mSignalingClientHandle)); + EXPECT_EQ(expectedStatus, signalingClientConnectSync(mSignalingClientHandle)); + + // Attempt to send a message should fail + SignalingMessage signalingMessage; + signalingMessage.version = SIGNALING_MESSAGE_CURRENT_VERSION; + signalingMessage.messageType = SIGNALING_MESSAGE_TYPE_OFFER; + STRCPY(signalingMessage.peerClientId, TEST_SIGNALING_MASTER_CLIENT_ID); + MEMSET(signalingMessage.payload, 'A', 100); + signalingMessage.payload[100] = '\0'; + signalingMessage.payloadLen = 0; + signalingMessage.correlationId[0] = '\0'; + EXPECT_EQ(expectedStatus, signalingClientSendMessageSync(mSignalingClientHandle, &signalingMessage)); deinitializeSignalingClient(); } TEST_F(SignalingApiTest, signalingClientGetIceConfigInfoCount) { - if (!mAccessKeyIdSet) { - return; - } - STATUS expectedStatus; UINT32 count; @@ -99,10 +116,6 @@ TEST_F(SignalingApiTest, signalingClientGetIceConfigInfoCount) TEST_F(SignalingApiTest, signalingClientGetIceConfigInfo) { - if (!mAccessKeyIdSet) { - return; - } - UINT32 i, j, count; PIceConfigInfo pIceConfigInfo; @@ -139,18 +152,19 @@ TEST_F(SignalingApiTest, signalingClientGetIceConfigInfo) TEST_F(SignalingApiTest, signalingClientGetCurrentState) { - if (!mAccessKeyIdSet) { - return; - } + STATUS expectedStatus; + SIGNALING_CLIENT_STATE state, expectedState; - SIGNALING_CLIENT_STATE state; initializeSignalingClient(); EXPECT_NE(STATUS_SUCCESS, signalingClientGetCurrentState(INVALID_SIGNALING_CLIENT_HANDLE_VALUE, &state)); EXPECT_NE(STATUS_SUCCESS, signalingClientGetCurrentState(mSignalingClientHandle, NULL)); EXPECT_NE(STATUS_SUCCESS, signalingClientGetCurrentState(INVALID_SIGNALING_CLIENT_HANDLE_VALUE, NULL)); - EXPECT_EQ(STATUS_SUCCESS, signalingClientGetCurrentState(mSignalingClientHandle, &state)); - EXPECT_EQ(SIGNALING_CLIENT_STATE_READY, state); + expectedStatus = mAccessKeyIdSet ? STATUS_SUCCESS : STATUS_NULL_ARG; + EXPECT_EQ(expectedStatus, signalingClientGetCurrentState(mSignalingClientHandle, &state)); + + expectedState = mAccessKeyIdSet ? SIGNALING_CLIENT_STATE_READY : SIGNALING_CLIENT_STATE_UNKNOWN; + EXPECT_EQ(expectedState, state); deinitializeSignalingClient(); } diff --git a/tst/WebRTCClientTestFixture.cpp b/tst/WebRTCClientTestFixture.cpp index d297d93677..876852de96 100644 --- a/tst/WebRTCClientTestFixture.cpp +++ b/tst/WebRTCClientTestFixture.cpp @@ -155,34 +155,34 @@ bool WebRtcClientTestBase::connectTwoPeers(PRtcPeerConnection offerPc, PRtcPeerC if (candidateStr != NULL) { std::thread([customData] (std::string candidate) { RtcIceCandidateInit iceCandidate; - EXPECT_EQ(deserializeRtcIceCandidateInit((PCHAR) candidate.c_str(), STRLEN(candidate.c_str()), &iceCandidate), STATUS_SUCCESS); - EXPECT_EQ(addIceCandidate((PRtcPeerConnection) customData, iceCandidate.candidate), STATUS_SUCCESS); + EXPECT_EQ(STATUS_SUCCESS, deserializeRtcIceCandidateInit((PCHAR) candidate.c_str(), STRLEN(candidate.c_str()), &iceCandidate)); + EXPECT_EQ(STATUS_SUCCESS, addIceCandidate((PRtcPeerConnection) customData, iceCandidate.candidate)); }, std::string(candidateStr)).detach(); } }; - EXPECT_EQ(peerConnectionOnIceCandidate(offerPc, (UINT64) answerPc, onICECandidateHdlr), STATUS_SUCCESS); - EXPECT_EQ(peerConnectionOnIceCandidate(answerPc, (UINT64) offerPc, onICECandidateHdlr), STATUS_SUCCESS); + EXPECT_EQ(STATUS_SUCCESS, peerConnectionOnIceCandidate(offerPc, (UINT64) answerPc, onICECandidateHdlr)); + EXPECT_EQ(STATUS_SUCCESS, peerConnectionOnIceCandidate(answerPc, (UINT64) offerPc, onICECandidateHdlr)); auto onICEConnectionStateChangeHdlr = [](UINT64 customData, RTC_PEER_CONNECTION_STATE newState) -> void { ATOMIC_INCREMENT((PSIZE_T)customData + newState); }; - EXPECT_EQ(peerConnectionOnConnectionStateChange(offerPc, (UINT64) this->stateChangeCount, onICEConnectionStateChangeHdlr), STATUS_SUCCESS); - EXPECT_EQ(peerConnectionOnConnectionStateChange(answerPc, (UINT64) this->stateChangeCount, onICEConnectionStateChangeHdlr), STATUS_SUCCESS); + EXPECT_EQ(STATUS_SUCCESS, peerConnectionOnConnectionStateChange(offerPc, (UINT64) this->stateChangeCount, onICEConnectionStateChangeHdlr)); + EXPECT_EQ(STATUS_SUCCESS, peerConnectionOnConnectionStateChange(answerPc, (UINT64) this->stateChangeCount, onICEConnectionStateChangeHdlr)); - EXPECT_EQ(createOffer(offerPc, &sdp), STATUS_SUCCESS); - EXPECT_EQ(setLocalDescription(offerPc, &sdp), STATUS_SUCCESS); - EXPECT_EQ(setRemoteDescription(answerPc, &sdp), STATUS_SUCCESS); + EXPECT_EQ(STATUS_SUCCESS, createOffer(offerPc, &sdp)); + EXPECT_EQ(STATUS_SUCCESS, setLocalDescription(offerPc, &sdp)); + EXPECT_EQ(STATUS_SUCCESS, setRemoteDescription(answerPc, &sdp)); // Validate the cert fingerprint if we are asked to do so if (pOfferCertFingerprint != NULL) { EXPECT_NE((PCHAR) NULL, STRSTR(sdp.sdp, pOfferCertFingerprint)); } - EXPECT_EQ(createAnswer(answerPc, &sdp), STATUS_SUCCESS); - EXPECT_EQ(setLocalDescription(answerPc, &sdp), STATUS_SUCCESS); - EXPECT_EQ(setRemoteDescription(offerPc, &sdp), STATUS_SUCCESS); + EXPECT_EQ(STATUS_SUCCESS, createAnswer(answerPc, &sdp)); + EXPECT_EQ(STATUS_SUCCESS, setLocalDescription(answerPc, &sdp)); + EXPECT_EQ(STATUS_SUCCESS, setRemoteDescription(offerPc, &sdp)); if (pAnswerCertFingerprint != NULL) { EXPECT_NE((PCHAR) NULL, STRSTR(sdp.sdp, pAnswerCertFingerprint)); @@ -200,14 +200,14 @@ void WebRtcClientTestBase::addTrackToPeerConnection(PRtcPeerConnection pRtcPeerC { MEMSET(track, 0x00, SIZEOF(RtcMediaStreamTrack)); - EXPECT_EQ(addSupportedCodec(pRtcPeerConnection, codec), STATUS_SUCCESS); + EXPECT_EQ(STATUS_SUCCESS, addSupportedCodec(pRtcPeerConnection, codec)); track->kind = kind; track->codec = codec; - EXPECT_EQ(generateJSONSafeString(track->streamId, MAX_MEDIA_STREAM_ID_LEN), STATUS_SUCCESS); - EXPECT_EQ(generateJSONSafeString(track->trackId, MAX_MEDIA_STREAM_ID_LEN), STATUS_SUCCESS); + EXPECT_EQ(STATUS_SUCCESS, generateJSONSafeString(track->streamId, MAX_MEDIA_STREAM_ID_LEN)); + EXPECT_EQ(STATUS_SUCCESS, generateJSONSafeString(track->trackId, MAX_MEDIA_STREAM_ID_LEN)); - EXPECT_EQ(addTransceiver(pRtcPeerConnection, track, NULL, transceiver), STATUS_SUCCESS); + EXPECT_EQ(STATUS_SUCCESS, addTransceiver(pRtcPeerConnection, track, NULL, transceiver)); } void WebRtcClientTestBase::getIceServers(PRtcConfiguration pRtcConfiguration) @@ -216,13 +216,13 @@ void WebRtcClientTestBase::getIceServers(PRtcConfiguration pRtcConfiguration) PIceConfigInfo pIceConfigInfo; // Assume signaling client is already created - EXPECT_EQ(signalingClientGetIceConfigInfoCount(mSignalingClientHandle, &iceConfigCount), STATUS_SUCCESS); + EXPECT_EQ(STATUS_SUCCESS, signalingClientGetIceConfigInfoCount(mSignalingClientHandle, &iceConfigCount)); // Set the STUN server SNPRINTF(pRtcConfiguration->iceServers[0].urls, MAX_ICE_CONFIG_URI_LEN, KINESIS_VIDEO_STUN_URL, TEST_DEFAULT_REGION); for (uriCount = 0, i = 0; i < iceConfigCount; i++) { - EXPECT_EQ(signalingClientGetIceConfigInfo(mSignalingClientHandle, i, &pIceConfigInfo), STATUS_SUCCESS); + EXPECT_EQ(STATUS_SUCCESS, signalingClientGetIceConfigInfo(mSignalingClientHandle, i, &pIceConfigInfo)); for (j = 0; j < pIceConfigInfo->uriCount; j++) { STRNCPY(pRtcConfiguration->iceServers[uriCount + 1].urls, pIceConfigInfo->uris[j], MAX_ICE_CONFIG_URI_LEN); STRNCPY(pRtcConfiguration->iceServers[uriCount + 1].credential, pIceConfigInfo->password, MAX_ICE_CONFIG_CREDENTIAL_LEN); diff --git a/tst/WebRTCClientTestFixture.h b/tst/WebRTCClientTestFixture.h index ea3cca886c..52eeb0d850 100644 --- a/tst/WebRTCClientTestFixture.h +++ b/tst/WebRTCClientTestFixture.h @@ -99,6 +99,7 @@ class WebRtcClientTestBase : public ::testing::Test { if (mAccessKeyIdSet) { EXPECT_EQ(STATUS_SUCCESS, retStatus); } else { + mSignalingClientHandle = INVALID_SIGNALING_CLIENT_HANDLE_VALUE; EXPECT_NE(STATUS_SUCCESS, retStatus); } diff --git a/tst/suppressions/TSAN.supp b/tst/suppressions/TSAN.supp index d93cb88643..fa91e057e2 100644 --- a/tst/suppressions/TSAN.supp +++ b/tst/suppressions/TSAN.supp @@ -307,6 +307,24 @@ race:connectSignalingChannelLws # #11 void testing::internal::HandleSehExceptionsInMethodIfSupported(testing::Test*, void (testing::Test::*)(), char const*) (webrtc_client_test+0x000000672819) deadlock:connectSignalingChannelLws +# WARNING: ThreadSanitizer: data race (pid=21078) +# Write of size 8 at 0x7b08000200a0 by thread T16 (mutexes: write M266973569391553680, write M274010443809316528): +# #0 free /tmp/final/llvm.src/projects/compiler-rt/lib/tsan/rtl/tsan_interceptors.cc:707:3 (webrtc_client_test+0x4db8f4) +# #1 _realloc (libwebsockets.so.15+0xa80d) +# #2 lwsListenerHandler /home/travis/build/awslabs/amazon-kinesis-video-streams-webrtc-sdk-c/src/source/Signaling/LwsApiCalls.c:1373:5 (libkvsWebrtcSignalingClient.so+0x19666) +# +# Previous write of size 8 at 0x7b08000200a0 by main thread (mutexes: write M273728968832621472): +# #0 realloc /tmp/final/llvm.src/projects/compiler-rt/lib/tsan/rtl/tsan_interceptors.cc:694:5 (webrtc_client_test+0x4db7e7) +# #1 _realloc (libwebsockets.so.15+0xa7dd) +# #2 terminateConnectionWithStatus /home/travis/build/awslabs/amazon-kinesis-video-streams-webrtc-sdk-c/src/source/Signaling/LwsApiCalls.c:1827:5 (libkvsWebrtcSignalingClient.so+0x18709) +# #3 terminateLwsListenerLoop /home/travis/build/awslabs/amazon-kinesis-video-streams-webrtc-sdk-c/src/source/Signaling/LwsApiCalls.c:1885:9 (libkvsWebrtcSignalingClient.so+0x1ae86) +# #4 terminateOngoingOperations /home/travis/build/awslabs/amazon-kinesis-video-streams-webrtc-sdk-c/src/source/Signaling/Signaling.c:262:5 (libkvsWebrtcSignalingClient.so+0x1d6c5) +# #5 signalingDeleteSync /home/travis/build/awslabs/amazon-kinesis-video-streams-webrtc-sdk-c/src/source/Signaling/Signaling.c:416:5 (libkvsWebrtcSignalingClient.so+0x1ea4d) +# #6 signalingClientDeleteSync /home/travis/build/awslabs/amazon-kinesis-video-streams-webrtc-sdk-c/src/source/Signaling/Client.c:99:5 (libkvsWebrtcSignalingClient.so+0xc50e) +# #7 com::amazonaws::kinesis::video::webrtcclient::SignalingApiFunctionalityTest_deleteChannelCreatedWithArn_Test::TestBody() /home/travis/build/awslabs/amazon-kinesis-video-streams-webrtc-sdk-c/tst/SignalingApiFunctionalityTest.cpp:1835:5 (webrtc_client_test+0x638aa7) +# #8 void testing::internal::HandleSehExceptionsInMethodIfSupported(testing::Test*, void (testing::Test::*)(), char const*) (webrtc_client_test+0x6cc5dd) +race:terminateConnectionWithStatus + # test code synchronized by sleep race:PeerConnectionFunctionalityTest_freeTurnDueToP2PFoundBeforeTurnEstablished_Test race:PeerConnectionFunctionalityTest_freeTurnDueToP2PFoundAfterTurnEstablished