From f88fd920ef58a743b4175e147c7be9993b2859ad Mon Sep 17 00:00:00 2001 From: James Delaplane Date: Thu, 12 May 2022 12:37:48 -0700 Subject: [PATCH 1/8] recreate signaling client & lws_context whenever a significant error has occurred --- samples/Common.c | 26 ++++++++++++++++++++++---- 1 file changed, 22 insertions(+), 4 deletions(-) diff --git a/samples/Common.c b/samples/Common.c index c7b32b09ad..780766834c 100644 --- a/samples/Common.c +++ b/samples/Common.c @@ -12,6 +12,15 @@ VOID sigintHandler(INT32 sigNum) } } +STATUS signalingCallFailed(STATUS status) { + return (STATUS_SIGNALING_GET_TOKEN_CALL_FAILED == status || + STATUS_SIGNALING_DESCRIBE_CALL_FAILED == status || + STATUS_SIGNALING_CREATE_CALL_FAILED == status || + STATUS_SIGNALING_GET_ENDPOINT_CALL_FAILED == status || + STATUS_SIGNALING_GET_ICE_CONFIG_CALL_FAILED == status || + STATUS_SIGNALING_CONNECT_CALL_FAILED == status); +} + VOID onDataChannelMessage(UINT64 customData, PRtcDataChannel pDataChannel, BOOL isBinary, PBYTE pMessage, UINT32 pMessageLen) { UNUSED_PARAM(customData); @@ -1166,10 +1175,19 @@ STATUS sessionCleanupWait(PSampleConfiguration pSampleConfiguration) } // Check if we need to re-create the signaling client on-the-fly - if (ATOMIC_LOAD_BOOL(&pSampleConfiguration->recreateSignalingClient) && - STATUS_SUCCEEDED(signalingClientFetchSync(pSampleConfiguration->signalingClientHandle))) { - // Re-set the variable again - ATOMIC_STORE_BOOL(&pSampleConfiguration->recreateSignalingClient, FALSE); + if (ATOMIC_LOAD_BOOL(&pSampleConfiguration->recreateSignalingClient)) { + retStatus = signalingClientFetchSync(pSampleConfiguration->signalingClientHandle); + if(STATUS_SUCCEEDED(retStatus)) { + // Re-set the variable again + ATOMIC_STORE_BOOL(&pSampleConfiguration->recreateSignalingClient, FALSE); + } + else if(signalingCallFailed(retStatus)) { + printf("[KVS Common] recreating Signaling Client\n"); + freeSignalingClient(&pSampleConfiguration->signalingClientHandle); + createSignalingClientSync(&pSampleConfiguration->clientInfo, &pSampleConfiguration->channelInfo, + &pSampleConfiguration->signalingClientCallbacks, pSampleConfiguration->pCredentialProvider, + &pSampleConfiguration->signalingClientHandle); + } } // Check the signaling client state and connect if needed From 0c0b3d6e92708737ac99279110464b710f198bc1 Mon Sep 17 00:00:00 2001 From: Jeremy Gunawan Date: Fri, 10 Feb 2023 16:09:41 -0800 Subject: [PATCH 2/8] clang-format --- samples/Common.c | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/samples/Common.c b/samples/Common.c index 780766834c..85df0696c0 100644 --- a/samples/Common.c +++ b/samples/Common.c @@ -12,13 +12,11 @@ VOID sigintHandler(INT32 sigNum) } } -STATUS signalingCallFailed(STATUS status) { - return (STATUS_SIGNALING_GET_TOKEN_CALL_FAILED == status || - STATUS_SIGNALING_DESCRIBE_CALL_FAILED == status || - STATUS_SIGNALING_CREATE_CALL_FAILED == status || - STATUS_SIGNALING_GET_ENDPOINT_CALL_FAILED == status || - STATUS_SIGNALING_GET_ICE_CONFIG_CALL_FAILED == status || - STATUS_SIGNALING_CONNECT_CALL_FAILED == status); +STATUS signalingCallFailed(STATUS status) +{ + return (STATUS_SIGNALING_GET_TOKEN_CALL_FAILED == status || STATUS_SIGNALING_DESCRIBE_CALL_FAILED == status || + STATUS_SIGNALING_CREATE_CALL_FAILED == status || STATUS_SIGNALING_GET_ENDPOINT_CALL_FAILED == status || + STATUS_SIGNALING_GET_ICE_CONFIG_CALL_FAILED == status || STATUS_SIGNALING_CONNECT_CALL_FAILED == status); } VOID onDataChannelMessage(UINT64 customData, PRtcDataChannel pDataChannel, BOOL isBinary, PBYTE pMessage, UINT32 pMessageLen) @@ -1177,11 +1175,10 @@ STATUS sessionCleanupWait(PSampleConfiguration pSampleConfiguration) // Check if we need to re-create the signaling client on-the-fly if (ATOMIC_LOAD_BOOL(&pSampleConfiguration->recreateSignalingClient)) { retStatus = signalingClientFetchSync(pSampleConfiguration->signalingClientHandle); - if(STATUS_SUCCEEDED(retStatus)) { + if (STATUS_SUCCEEDED(retStatus)) { // Re-set the variable again ATOMIC_STORE_BOOL(&pSampleConfiguration->recreateSignalingClient, FALSE); - } - else if(signalingCallFailed(retStatus)) { + } else if (signalingCallFailed(retStatus)) { printf("[KVS Common] recreating Signaling Client\n"); freeSignalingClient(&pSampleConfiguration->signalingClientHandle); createSignalingClientSync(&pSampleConfiguration->clientInfo, &pSampleConfiguration->channelInfo, From 2d784b0b98aa8ebc52cb49f35d513bcf0f4bb936 Mon Sep 17 00:00:00 2001 From: James Delaplane Date: Thu, 12 May 2022 12:37:48 -0700 Subject: [PATCH 3/8] recreate signaling client & lws_context whenever a significant error has occurred --- samples/Common.c | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/samples/Common.c b/samples/Common.c index 85df0696c0..780766834c 100644 --- a/samples/Common.c +++ b/samples/Common.c @@ -12,11 +12,13 @@ VOID sigintHandler(INT32 sigNum) } } -STATUS signalingCallFailed(STATUS status) -{ - return (STATUS_SIGNALING_GET_TOKEN_CALL_FAILED == status || STATUS_SIGNALING_DESCRIBE_CALL_FAILED == status || - STATUS_SIGNALING_CREATE_CALL_FAILED == status || STATUS_SIGNALING_GET_ENDPOINT_CALL_FAILED == status || - STATUS_SIGNALING_GET_ICE_CONFIG_CALL_FAILED == status || STATUS_SIGNALING_CONNECT_CALL_FAILED == status); +STATUS signalingCallFailed(STATUS status) { + return (STATUS_SIGNALING_GET_TOKEN_CALL_FAILED == status || + STATUS_SIGNALING_DESCRIBE_CALL_FAILED == status || + STATUS_SIGNALING_CREATE_CALL_FAILED == status || + STATUS_SIGNALING_GET_ENDPOINT_CALL_FAILED == status || + STATUS_SIGNALING_GET_ICE_CONFIG_CALL_FAILED == status || + STATUS_SIGNALING_CONNECT_CALL_FAILED == status); } VOID onDataChannelMessage(UINT64 customData, PRtcDataChannel pDataChannel, BOOL isBinary, PBYTE pMessage, UINT32 pMessageLen) @@ -1175,10 +1177,11 @@ STATUS sessionCleanupWait(PSampleConfiguration pSampleConfiguration) // Check if we need to re-create the signaling client on-the-fly if (ATOMIC_LOAD_BOOL(&pSampleConfiguration->recreateSignalingClient)) { retStatus = signalingClientFetchSync(pSampleConfiguration->signalingClientHandle); - if (STATUS_SUCCEEDED(retStatus)) { + if(STATUS_SUCCEEDED(retStatus)) { // Re-set the variable again ATOMIC_STORE_BOOL(&pSampleConfiguration->recreateSignalingClient, FALSE); - } else if (signalingCallFailed(retStatus)) { + } + else if(signalingCallFailed(retStatus)) { printf("[KVS Common] recreating Signaling Client\n"); freeSignalingClient(&pSampleConfiguration->signalingClientHandle); createSignalingClientSync(&pSampleConfiguration->clientInfo, &pSampleConfiguration->channelInfo, From e85d52666905c7ca94e150680cb3e0276ecb2531 Mon Sep 17 00:00:00 2001 From: Jeremy Gunawan Date: Fri, 10 Feb 2023 16:12:41 -0800 Subject: [PATCH 4/8] Clang --- samples/Common.c | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/samples/Common.c b/samples/Common.c index 780766834c..85df0696c0 100644 --- a/samples/Common.c +++ b/samples/Common.c @@ -12,13 +12,11 @@ VOID sigintHandler(INT32 sigNum) } } -STATUS signalingCallFailed(STATUS status) { - return (STATUS_SIGNALING_GET_TOKEN_CALL_FAILED == status || - STATUS_SIGNALING_DESCRIBE_CALL_FAILED == status || - STATUS_SIGNALING_CREATE_CALL_FAILED == status || - STATUS_SIGNALING_GET_ENDPOINT_CALL_FAILED == status || - STATUS_SIGNALING_GET_ICE_CONFIG_CALL_FAILED == status || - STATUS_SIGNALING_CONNECT_CALL_FAILED == status); +STATUS signalingCallFailed(STATUS status) +{ + return (STATUS_SIGNALING_GET_TOKEN_CALL_FAILED == status || STATUS_SIGNALING_DESCRIBE_CALL_FAILED == status || + STATUS_SIGNALING_CREATE_CALL_FAILED == status || STATUS_SIGNALING_GET_ENDPOINT_CALL_FAILED == status || + STATUS_SIGNALING_GET_ICE_CONFIG_CALL_FAILED == status || STATUS_SIGNALING_CONNECT_CALL_FAILED == status); } VOID onDataChannelMessage(UINT64 customData, PRtcDataChannel pDataChannel, BOOL isBinary, PBYTE pMessage, UINT32 pMessageLen) @@ -1177,11 +1175,10 @@ STATUS sessionCleanupWait(PSampleConfiguration pSampleConfiguration) // Check if we need to re-create the signaling client on-the-fly if (ATOMIC_LOAD_BOOL(&pSampleConfiguration->recreateSignalingClient)) { retStatus = signalingClientFetchSync(pSampleConfiguration->signalingClientHandle); - if(STATUS_SUCCEEDED(retStatus)) { + if (STATUS_SUCCEEDED(retStatus)) { // Re-set the variable again ATOMIC_STORE_BOOL(&pSampleConfiguration->recreateSignalingClient, FALSE); - } - else if(signalingCallFailed(retStatus)) { + } else if (signalingCallFailed(retStatus)) { printf("[KVS Common] recreating Signaling Client\n"); freeSignalingClient(&pSampleConfiguration->signalingClientHandle); createSignalingClientSync(&pSampleConfiguration->clientInfo, &pSampleConfiguration->channelInfo, From f8cbda4eb7550f217b7c86872b00a14c7f8588c7 Mon Sep 17 00:00:00 2001 From: James Delaplane Date: Mon, 1 Aug 2022 13:56:03 -0700 Subject: [PATCH 5/8] adding local and remote null checks --- src/source/Ice/IceAgent.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/source/Ice/IceAgent.c b/src/source/Ice/IceAgent.c index 6f363e1819..c0e430875a 100644 --- a/src/source/Ice/IceAgent.c +++ b/src/source/Ice/IceAgent.c @@ -1229,6 +1229,7 @@ STATUS iceCandidatePairCheckConnection(PStunPacket pStunBindingRequest, PIceAgen UINT32 checkSum = 0; CHK(pStunBindingRequest != NULL && pIceAgent != NULL && pIceCandidatePair != NULL, STATUS_NULL_ARG); + CHK(pIceCandidatePair->local != NULL && pIceCandidatePair->remote != NULL, STATUS_NULL_ARG); CHK_STATUS(getStunAttribute(pStunBindingRequest, STUN_ATTRIBUTE_TYPE_PRIORITY, (PStunAttributeHeader*) &pStunAttributePriority)); CHK(pStunAttributePriority != NULL, STATUS_INVALID_ARG); From 9eb10dde30feb59c1037def18bf33c91109311d2 Mon Sep 17 00:00:00 2001 From: James Delaplane Date: Wed, 18 May 2022 12:24:17 -0700 Subject: [PATCH 6/8] More verbose and debug logging for ice & turn --- src/source/Ice/IceAgent.c | 50 +++++++++++++++++++++------ src/source/Ice/IceAgentStateMachine.c | 1 + src/source/Ice/TurnConnection.c | 15 +++++++- 3 files changed, 55 insertions(+), 11 deletions(-) diff --git a/src/source/Ice/IceAgent.c b/src/source/Ice/IceAgent.c index c0e430875a..3386e57c66 100644 --- a/src/source/Ice/IceAgent.c +++ b/src/source/Ice/IceAgent.c @@ -1234,6 +1234,20 @@ STATUS iceCandidatePairCheckConnection(PStunPacket pStunBindingRequest, PIceAgen CHK(pStunAttributePriority != NULL, STATUS_INVALID_ARG); + if(pIceCandidatePair->local->ipAddress.family == KVS_IP_FAMILY_TYPE_IPV4) { + DLOGD("remote ip:%u.%u.%u.%u, port:%u, local ip:%u.%u.%u.%u, port:%u", pIceCandidatePair->remote->ipAddress.address[0], + pIceCandidatePair->remote->ipAddress.address[1], + pIceCandidatePair->remote->ipAddress.address[2], + pIceCandidatePair->remote->ipAddress.address[3], + pIceCandidatePair->remote->ipAddress.address[0], + pIceCandidatePair->remote->ipAddress.port, + pIceCandidatePair->local->ipAddress.address[1], + pIceCandidatePair->local->ipAddress.address[2], + pIceCandidatePair->local->ipAddress.address[3], + pIceCandidatePair->local->ipAddress.address[0], + pIceCandidatePair->local->ipAddress.port); + } + // update priority and transaction id pStunAttributePriority->priority = pIceCandidatePair->local->priority; CHK_STATUS(iceUtilsGenerateTransactionId(pStunBindingRequest->header.transactionId, ARRAY_SIZE(pStunBindingRequest->header.transactionId))); @@ -1370,6 +1384,7 @@ STATUS iceAgentSendSrflxCandidateRequest(PIceAgent pIceAgent) case ICE_CANDIDATE_TYPE_SERVER_REFLEXIVE: pIceServer = &(pIceAgent->iceServers[pCandidate->iceServerIndex]); if (pIceServer->ipAddress.family == pCandidate->ipAddress.family) { + transactionIdStoreInsert(pIceAgent->pStunBindingRequestTransactionIdStore, pBindingRequest->header.transactionId); checkSum = COMPUTE_CRC32(pBindingRequest->header.transactionId, ARRAY_SIZE(pBindingRequest->header.transactionId)); CHK_STATUS(iceAgentSendStunPacket(pBindingRequest, NULL, 0, pIceAgent, pCandidate, &pIceServer->ipAddress)); @@ -2248,11 +2263,13 @@ STATUS incomingRelayedDataHandler(UINT64 customData, PSocketConnection pSocketCo STATUS retStatus = STATUS_SUCCESS; PIceCandidate pRelayedCandidate = (PIceCandidate) customData; // this should be more than enough. Usually the number of channel data in each tcp message is around 4 - TurnChannelData turnChannelData[DEFAULT_TURN_CHANNEL_DATA_BUFFER_SIZE]; + TurnChannelData turnChannelData[DEFAULT_TURN_CHANNEL_DATA_BUFFER_SIZE] = {0}; UINT32 turnChannelDataCount = ARRAY_SIZE(turnChannelData), i = 0; CHK(pRelayedCandidate != NULL && pSocketConnection != NULL, STATUS_NULL_ARG); + + DLOGV("Candidate id: %s", pRelayedCandidate->id); CHK_STATUS(turnConnectionIncomingDataHandler(pRelayedCandidate->pTurnConnection, pBuffer, bufferLen, pSrc, pDest, turnChannelData, &turnChannelDataCount)); for (i = 0; i < turnChannelDataCount; ++i) { @@ -2283,6 +2300,7 @@ STATUS incomingDataHandler(UINT64 customData, PSocketConnection pSocketConnectio // for stun packets, first 8 bytes are 4 byte type and length, then 4 byte magic byte if ((bufferLen < 8 || !IS_STUN_PACKET(pBuffer)) && pIceAgent->iceAgentCallbacks.inboundPacketFn != NULL) { // release lock early + MUTEX_UNLOCK(pIceAgent->lock); locked = FALSE; pIceAgent->iceAgentCallbacks.inboundPacketFn(pIceAgent->iceAgentCallbacks.customData, pBuffer, bufferLen); @@ -2442,7 +2460,7 @@ STATUS handleStunPacket(PIceAgent pIceAgent, PBYTE pBuffer, UINT32 bufferLen, PS pIceAgent->rtcIceServerDiagnostics[pIceCandidate->iceServerIndex].totalResponsesReceived++; retStatus = hashTableGet(pIceAgent->requestTimestampDiagnostics, checkSum, &requestSentTime); if (retStatus != STATUS_SUCCESS) { - DLOGW("Unable to fetch request Timestamp from the hash table. No update to totalRoundTripTime (error code: 0x%08x)", retStatus); + DLOGW("Unable to fetch request Timestamp from the hash table. No update to totalRoundTripTime (error code: 0x%08x), stunBindingRequest", retStatus); } else { pIceAgent->rtcIceServerDiagnostics[pIceCandidate->iceServerIndex].totalRoundTripTime += GETTIME() - requestSentTime; CHK_STATUS(hashTableRemove(pIceAgent->requestTimestampDiagnostics, checkSum)); @@ -2470,6 +2488,7 @@ STATUS handleStunPacket(PIceAgent pIceAgent, PBYTE pBuffer, UINT32 bufferLen, PS "Cannot find candidate pair with local candidate %s and remote candidate %s. Dropping STUN binding success response", ipAddrStr2, ipAddrStr); } + DLOGV("Pair binding response! %s %s", pIceCandidatePair->local->id, pIceCandidatePair->remote->id); retStatus = hashTableGet(pIceCandidatePair->requestSentTime, checkSum, &requestSentTime); if (retStatus != STATUS_SUCCESS) { DLOGW("Unable to fetch request Timestamp from the hash table. No update to RTT for the pair (error code: 0x%08x)", retStatus); @@ -2486,7 +2505,7 @@ STATUS handleStunPacket(PIceAgent pIceAgent, PBYTE pBuffer, UINT32 bufferLen, PS pIceAgent->rtcIceServerDiagnostics[pIceCandidatePair->local->iceServerIndex].totalResponsesReceived++; retStatus = hashTableGet(pIceAgent->requestTimestampDiagnostics, checkSum, &requestSentTime); if (retStatus != STATUS_SUCCESS) { - DLOGW("Unable to fetch request Timestamp from the hash table. No update to totalRoundTripTime (error code: 0x%08x)", retStatus); + DLOGW("Unable to fetch request Timestamp from the hash table. No update to totalRoundTripTime (error code: 0x%08x), typeRelayed", retStatus); } else { pIceAgent->rtcIceServerDiagnostics[pIceCandidatePair->local->iceServerIndex].totalRoundTripTime += GETTIME() - requestSentTime; CHK_STATUS(hashTableRemove(pIceAgent->requestTimestampDiagnostics, checkSum)); @@ -2514,10 +2533,11 @@ STATUS handleStunPacket(PIceAgent pIceAgent, PBYTE pBuffer, UINT32 bufferLen, PS } if (pIceCandidatePair->state != ICE_CANDIDATE_PAIR_STATE_SUCCEEDED) { + DLOGV("Pair succeeded! %s %s", pIceCandidatePair->local->id, pIceCandidatePair->remote->id); pIceCandidatePair->state = ICE_CANDIDATE_PAIR_STATE_SUCCEEDED; retStatus = hashTableGet(pIceCandidatePair->requestSentTime, checkSum, &requestSentTime); if (retStatus != STATUS_SUCCESS) { - DLOGW("Unable to fetch request Timestamp from the hash table. No update to totalRoundTripTime (error code: 0x%08x)", retStatus); + DLOGW("Unable to fetch request Timestamp from the hash table. No update to totalRoundTripTime (error code: 0x%08x), stateSucceeded", retStatus); } else { pIceCandidatePair->roundTripTime = GETTIME() - requestSentTime; DLOGD("Ice candidate pair %s_%s is connected. Round trip time: %" PRIu64 "ms", pIceCandidatePair->local->id, @@ -2538,12 +2558,22 @@ STATUS handleStunPacket(PIceAgent pIceAgent, PBYTE pBuffer, UINT32 bufferLen, PS break; default: - CHK_STATUS(hexEncode(pBuffer, bufferLen, NULL, &hexStrLen)); - hexStr = MEMCALLOC(1, hexStrLen * SIZEOF(CHAR)); - CHK(hexStr != NULL, STATUS_NOT_ENOUGH_MEMORY); - CHK_STATUS(hexEncode(pBuffer, bufferLen, hexStr, &hexStrLen)); - DLOGW("Dropping unrecognized STUN packet. Packet type: 0x%02x. Packet content: \n\t%s", stunPacketType, hexStr); - SAFE_MEMFREE(hexStr); + if(!IS_STUN_PACKET(pBuffer)) { + CHK_STATUS(hexEncode(pBuffer, bufferLen, NULL, &hexStrLen)); + hexStr = MEMCALLOC(1, hexStrLen * SIZEOF(CHAR)); + CHK(hexStr != NULL, STATUS_NOT_ENOUGH_MEMORY); + CHK_STATUS(hexEncode(pBuffer, bufferLen, hexStr, &hexStrLen)); + DLOGW("Dropping unrecognized STUN packet. Packet type: 0x%02x. Packet content: \n\t%s", stunPacketType, hexStr); + SAFE_MEMFREE(hexStr); + } + else if(STUN_PACKET_IS_TYPE_ERROR(pBuffer)) { + CHK_STATUS(hexEncode(pBuffer, bufferLen, NULL, &hexStrLen)); + hexStr = MEMCALLOC(1, hexStrLen * SIZEOF(CHAR)); + CHK(hexStr != NULL, STATUS_NOT_ENOUGH_MEMORY); + CHK_STATUS(hexEncode(pBuffer, bufferLen, hexStr, &hexStrLen)); + DLOGW("Error STUN packet. Packet type: 0x%02x. Packet content: \n\t%s", stunPacketType, hexStr); + SAFE_MEMFREE(hexStr); + } break; } diff --git a/src/source/Ice/IceAgentStateMachine.c b/src/source/Ice/IceAgentStateMachine.c index ad9b6f2ad3..b9c2c293fa 100644 --- a/src/source/Ice/IceAgentStateMachine.c +++ b/src/source/Ice/IceAgentStateMachine.c @@ -226,6 +226,7 @@ STATUS fromCheckConnectionIceAgentState(UINT64 customData, PUINT64 pState) CHK_STATUS(doubleListGetHeadNode(pIceAgent->iceCandidatePairs, &pCurNode)); while (pCurNode != NULL && !connectedCandidatePairFound) { pIceCandidatePair = (PIceCandidatePair) pCurNode->data; + DLOGD("Checking pair: %s %s, state: %d", pIceCandidatePair->local->id, pIceCandidatePair->remote->id, pIceCandidatePair->state); pCurNode = pCurNode->pNext; if (pIceCandidatePair->state == ICE_CANDIDATE_PAIR_STATE_SUCCEEDED) { diff --git a/src/source/Ice/TurnConnection.c b/src/source/Ice/TurnConnection.c index cf91e06348..9690f61991 100644 --- a/src/source/Ice/TurnConnection.c +++ b/src/source/Ice/TurnConnection.c @@ -352,6 +352,7 @@ STATUS turnConnectionHandleStunError(PTurnConnection pTurnConnection, PBYTE pBuf MUTEX_LOCK(pTurnConnection->lock); locked = TRUE; + stunPacketType = (UINT16) getInt16(*((PUINT16) pBuffer)); /* we could get errors like expired nonce after sending the deallocation packet. The allocate would have been * deallocated even if the response is error response, and if we try to deallocate again we would get invalid * allocation error. Therefore if we get an error after we've sent the deallocation packet, consider the @@ -450,9 +451,10 @@ STATUS turnConnectionHandleChannelData(PTurnConnection pTurnConnection, PBYTE pB STATUS retStatus = STATUS_SUCCESS; BOOL locked = FALSE; - UINT32 turnChannelDataCount = 0; + UINT32 turnChannelDataCount = 0, hexStrLen = 0; UINT16 channelNumber = 0; PTurnPeer pTurnPeer = NULL; + PCHAR hexStr = NULL; CHK(pTurnConnection != NULL && pChannelData != NULL && pChannelDataCount != NULL && pProcessedDataLen != NULL, STATUS_NULL_ARG); CHK(pBuffer != NULL && bufferLen > 0, STATUS_INVALID_ARG); @@ -478,7 +480,14 @@ STATUS turnConnectionHandleChannelData(PTurnConnection pTurnConnection, PBYTE pB } } else { + CHK_STATUS(hexEncode(pBuffer, bufferLen, NULL, &hexStrLen)); + hexStr = MEMCALLOC(1, hexStrLen * SIZEOF(CHAR)); + CHK(hexStr != NULL, STATUS_NOT_ENOUGH_MEMORY); + CHK_STATUS(hexEncode(pBuffer, bufferLen, hexStr, &hexStrLen)); + DLOGE("Turn connection does not have channel number, dumping payload: %s", hexStr); turnChannelDataCount = 0; + + SAFE_MEMFREE(hexStr); } *pProcessedDataLen = bufferLen; @@ -493,6 +502,9 @@ STATUS turnConnectionHandleChannelData(PTurnConnection pTurnConnection, PBYTE pB CHK_LOG_ERR(retStatus); + if (hexStr != NULL) { + SAFE_MEMFREE(hexStr); + } if (locked) { MUTEX_UNLOCK(pTurnConnection->lock); } @@ -524,6 +536,7 @@ STATUS turnConnectionHandleChannelDataTcpMode(PTurnConnection pTurnConnection, P /* process only one channel data and return. Because channel data can be intermixed with STUN packet. * need to check remainingBufLen too because channel data could be incomplete. */ while (remainingBufLen != 0 && channelDataCount == 0) { + DLOGV("currRecvDataLen: %d", pTurnConnection->currRecvDataLen); if (pTurnConnection->currRecvDataLen != 0) { if (pTurnConnection->currRecvDataLen >= TURN_DATA_CHANNEL_SEND_OVERHEAD) { /* pTurnConnection->recvDataBuffer always has channel data start */ From 1278380727e461678bdcac121552fc2e1c25ec28 Mon Sep 17 00:00:00 2001 From: James Delaplane Date: Fri, 15 Apr 2022 11:50:09 -0700 Subject: [PATCH 7/8] Thread cancel on the media sender thread leads to memory leaks from writeFrame() not freeing all its heap usage --- samples/Common.c | 18 +++++++++++------- samples/Samples.h | 2 ++ samples/kvsWebRTCClientMaster.c | 20 ++++---------------- 3 files changed, 17 insertions(+), 23 deletions(-) diff --git a/samples/Common.c b/samples/Common.c index 85df0696c0..677aa56038 100644 --- a/samples/Common.c +++ b/samples/Common.c @@ -159,7 +159,9 @@ PVOID mediaSenderRoutine(PVOID customData) { STATUS retStatus = STATUS_SUCCESS; PSampleConfiguration pSampleConfiguration = (PSampleConfiguration) customData; - TID videoSenderTid = INVALID_TID_VALUE, audioSenderTid = INVALID_TID_VALUE; + CHK(pSampleConfiguration != NULL, STATUS_NULL_ARG); + pSampleConfiguration->videoSenderTid = INVALID_TID_VALUE; + pSampleConfiguration->audioSenderTid = INVALID_TID_VALUE; MUTEX_LOCK(pSampleConfiguration->sampleConfigurationObjLock); while (!ATOMIC_LOAD_BOOL(&pSampleConfiguration->connected) && !ATOMIC_LOAD_BOOL(&pSampleConfiguration->appTerminateFlag)) { @@ -170,19 +172,19 @@ PVOID mediaSenderRoutine(PVOID customData) CHK(!ATOMIC_LOAD_BOOL(&pSampleConfiguration->appTerminateFlag), retStatus); if (pSampleConfiguration->videoSource != NULL) { - THREAD_CREATE(&videoSenderTid, pSampleConfiguration->videoSource, (PVOID) pSampleConfiguration); + THREAD_CREATE(&pSampleConfiguration->videoSenderTid, pSampleConfiguration->videoSource, (PVOID) pSampleConfiguration); } if (pSampleConfiguration->audioSource != NULL) { - THREAD_CREATE(&audioSenderTid, pSampleConfiguration->audioSource, (PVOID) pSampleConfiguration); + THREAD_CREATE(&pSampleConfiguration->audioSenderTid, pSampleConfiguration->audioSource, (PVOID) pSampleConfiguration); } - if (videoSenderTid != INVALID_TID_VALUE) { - THREAD_JOIN(videoSenderTid, NULL); + if (pSampleConfiguration->videoSenderTid != INVALID_TID_VALUE) { + THREAD_JOIN(pSampleConfiguration->videoSenderTid, NULL); } - if (audioSenderTid != INVALID_TID_VALUE) { - THREAD_JOIN(audioSenderTid, NULL); + if (pSampleConfiguration->audioSenderTid != INVALID_TID_VALUE) { + THREAD_JOIN(pSampleConfiguration->audioSenderTid, NULL); } CleanUp: @@ -759,6 +761,8 @@ STATUS createSampleConfiguration(PCHAR channelName, SIGNALING_CHANNEL_ROLE_TYPE #endif pSampleConfiguration->mediaSenderTid = INVALID_TID_VALUE; + pSampleConfiguration->audioSenderTid = INVALID_TID_VALUE; + pSampleConfiguration->videoSenderTid = INVALID_TID_VALUE; pSampleConfiguration->signalingClientHandle = INVALID_SIGNALING_CLIENT_HANDLE_VALUE; pSampleConfiguration->sampleConfigurationObjLock = MUTEX_CREATE(TRUE); pSampleConfiguration->cvar = CVAR_CREATE(); diff --git a/samples/Samples.h b/samples/Samples.h index 2ff522270b..83ef0cb194 100644 --- a/samples/Samples.h +++ b/samples/Samples.h @@ -85,6 +85,8 @@ typedef struct { PBYTE pVideoFrameBuffer; UINT32 videoBufferSize; TID mediaSenderTid; + TID audioSenderTid; + TID videoSenderTid; TIMER_QUEUE_HANDLE timerQueueHandle; UINT32 iceCandidatePairStatsTimerId; SampleStreamingMediaType mediaType; diff --git a/samples/kvsWebRTCClientMaster.c b/samples/kvsWebRTCClientMaster.c index a8a4348ec2..18eb44b0db 100644 --- a/samples/kvsWebRTCClientMaster.c +++ b/samples/kvsWebRTCClientMaster.c @@ -128,20 +128,6 @@ INT32 main(INT32 argc, CHAR* argv[]) // Kick of the termination sequence ATOMIC_STORE_BOOL(&pSampleConfiguration->appTerminateFlag, TRUE); - if (IS_VALID_MUTEX_VALUE(pSampleConfiguration->sampleConfigurationObjLock)) { - MUTEX_LOCK(pSampleConfiguration->sampleConfigurationObjLock); - } - - // Cancel the media thread - if (pSampleConfiguration->mediaThreadStarted) { - DLOGD("Canceling media thread"); - THREAD_CANCEL(pSampleConfiguration->mediaSenderTid); - } - - if (IS_VALID_MUTEX_VALUE(pSampleConfiguration->sampleConfigurationObjLock)) { - MUTEX_UNLOCK(pSampleConfiguration->sampleConfigurationObjLock); - } - if (pSampleConfiguration->mediaSenderTid != INVALID_TID_VALUE) { THREAD_JOIN(pSampleConfiguration->mediaSenderTid, NULL); } @@ -166,6 +152,7 @@ INT32 main(INT32 argc, CHAR* argv[]) } } printf("[KVS Master] Cleanup done\n"); + CHK_LOG_ERR(retStatus); RESET_INSTRUMENTED_ALLOCATORS(); @@ -289,7 +276,7 @@ PVOID sendVideoPackets(PVOID args) } CleanUp: - + printf("[KVS Master] closing video thread"); CHK_LOG_ERR(retStatus); return (PVOID) (ULONG_PTR) retStatus; @@ -330,6 +317,7 @@ PVOID sendAudioPackets(PVOID args) printf("[KVS Master] MEMREALLOC(): operation returned status code: 0x%08x \n", STATUS_NOT_ENOUGH_MEMORY); goto CleanUp; } + pSampleConfiguration->audioBufferSize = frameSize; } frame.frameData = pSampleConfiguration->pAudioFrameBuffer; @@ -359,7 +347,7 @@ PVOID sendAudioPackets(PVOID args) } CleanUp: - + printf("[KVS Master] closing audio thread"); return (PVOID) (ULONG_PTR) retStatus; } From faeeaf51dbb7e1b6f4531305e3770dd40c79c6f9 Mon Sep 17 00:00:00 2001 From: Jeremy Gunawan Date: Fri, 10 Feb 2023 16:29:52 -0800 Subject: [PATCH 8/8] Clang again --- src/source/Ice/IceAgent.c | 35 ++++++++++++++++------------------- 1 file changed, 16 insertions(+), 19 deletions(-) diff --git a/src/source/Ice/IceAgent.c b/src/source/Ice/IceAgent.c index 3386e57c66..35075ea84f 100644 --- a/src/source/Ice/IceAgent.c +++ b/src/source/Ice/IceAgent.c @@ -1234,18 +1234,13 @@ STATUS iceCandidatePairCheckConnection(PStunPacket pStunBindingRequest, PIceAgen CHK(pStunAttributePriority != NULL, STATUS_INVALID_ARG); - if(pIceCandidatePair->local->ipAddress.family == KVS_IP_FAMILY_TYPE_IPV4) { + if (pIceCandidatePair->local->ipAddress.family == KVS_IP_FAMILY_TYPE_IPV4) { DLOGD("remote ip:%u.%u.%u.%u, port:%u, local ip:%u.%u.%u.%u, port:%u", pIceCandidatePair->remote->ipAddress.address[0], - pIceCandidatePair->remote->ipAddress.address[1], - pIceCandidatePair->remote->ipAddress.address[2], - pIceCandidatePair->remote->ipAddress.address[3], - pIceCandidatePair->remote->ipAddress.address[0], - pIceCandidatePair->remote->ipAddress.port, - pIceCandidatePair->local->ipAddress.address[1], - pIceCandidatePair->local->ipAddress.address[2], - pIceCandidatePair->local->ipAddress.address[3], - pIceCandidatePair->local->ipAddress.address[0], - pIceCandidatePair->local->ipAddress.port); + pIceCandidatePair->remote->ipAddress.address[1], pIceCandidatePair->remote->ipAddress.address[2], + pIceCandidatePair->remote->ipAddress.address[3], pIceCandidatePair->remote->ipAddress.address[0], + pIceCandidatePair->remote->ipAddress.port, pIceCandidatePair->local->ipAddress.address[1], + pIceCandidatePair->local->ipAddress.address[2], pIceCandidatePair->local->ipAddress.address[3], + pIceCandidatePair->local->ipAddress.address[0], pIceCandidatePair->local->ipAddress.port); } // update priority and transaction id @@ -1384,7 +1379,6 @@ STATUS iceAgentSendSrflxCandidateRequest(PIceAgent pIceAgent) case ICE_CANDIDATE_TYPE_SERVER_REFLEXIVE: pIceServer = &(pIceAgent->iceServers[pCandidate->iceServerIndex]); if (pIceServer->ipAddress.family == pCandidate->ipAddress.family) { - transactionIdStoreInsert(pIceAgent->pStunBindingRequestTransactionIdStore, pBindingRequest->header.transactionId); checkSum = COMPUTE_CRC32(pBindingRequest->header.transactionId, ARRAY_SIZE(pBindingRequest->header.transactionId)); CHK_STATUS(iceAgentSendStunPacket(pBindingRequest, NULL, 0, pIceAgent, pCandidate, &pIceServer->ipAddress)); @@ -2268,7 +2262,6 @@ STATUS incomingRelayedDataHandler(UINT64 customData, PSocketConnection pSocketCo CHK(pRelayedCandidate != NULL && pSocketConnection != NULL, STATUS_NULL_ARG); - DLOGV("Candidate id: %s", pRelayedCandidate->id); CHK_STATUS(turnConnectionIncomingDataHandler(pRelayedCandidate->pTurnConnection, pBuffer, bufferLen, pSrc, pDest, turnChannelData, &turnChannelDataCount)); @@ -2460,7 +2453,9 @@ STATUS handleStunPacket(PIceAgent pIceAgent, PBYTE pBuffer, UINT32 bufferLen, PS pIceAgent->rtcIceServerDiagnostics[pIceCandidate->iceServerIndex].totalResponsesReceived++; retStatus = hashTableGet(pIceAgent->requestTimestampDiagnostics, checkSum, &requestSentTime); if (retStatus != STATUS_SUCCESS) { - DLOGW("Unable to fetch request Timestamp from the hash table. No update to totalRoundTripTime (error code: 0x%08x), stunBindingRequest", retStatus); + DLOGW("Unable to fetch request Timestamp from the hash table. No update to totalRoundTripTime (error code: 0x%08x), " + "stunBindingRequest", + retStatus); } else { pIceAgent->rtcIceServerDiagnostics[pIceCandidate->iceServerIndex].totalRoundTripTime += GETTIME() - requestSentTime; CHK_STATUS(hashTableRemove(pIceAgent->requestTimestampDiagnostics, checkSum)); @@ -2505,7 +2500,8 @@ STATUS handleStunPacket(PIceAgent pIceAgent, PBYTE pBuffer, UINT32 bufferLen, PS pIceAgent->rtcIceServerDiagnostics[pIceCandidatePair->local->iceServerIndex].totalResponsesReceived++; retStatus = hashTableGet(pIceAgent->requestTimestampDiagnostics, checkSum, &requestSentTime); if (retStatus != STATUS_SUCCESS) { - DLOGW("Unable to fetch request Timestamp from the hash table. No update to totalRoundTripTime (error code: 0x%08x), typeRelayed", retStatus); + DLOGW("Unable to fetch request Timestamp from the hash table. No update to totalRoundTripTime (error code: 0x%08x), typeRelayed", + retStatus); } else { pIceAgent->rtcIceServerDiagnostics[pIceCandidatePair->local->iceServerIndex].totalRoundTripTime += GETTIME() - requestSentTime; CHK_STATUS(hashTableRemove(pIceAgent->requestTimestampDiagnostics, checkSum)); @@ -2537,7 +2533,9 @@ STATUS handleStunPacket(PIceAgent pIceAgent, PBYTE pBuffer, UINT32 bufferLen, PS pIceCandidatePair->state = ICE_CANDIDATE_PAIR_STATE_SUCCEEDED; retStatus = hashTableGet(pIceCandidatePair->requestSentTime, checkSum, &requestSentTime); if (retStatus != STATUS_SUCCESS) { - DLOGW("Unable to fetch request Timestamp from the hash table. No update to totalRoundTripTime (error code: 0x%08x), stateSucceeded", retStatus); + DLOGW( + "Unable to fetch request Timestamp from the hash table. No update to totalRoundTripTime (error code: 0x%08x), stateSucceeded", + retStatus); } else { pIceCandidatePair->roundTripTime = GETTIME() - requestSentTime; DLOGD("Ice candidate pair %s_%s is connected. Round trip time: %" PRIu64 "ms", pIceCandidatePair->local->id, @@ -2558,15 +2556,14 @@ STATUS handleStunPacket(PIceAgent pIceAgent, PBYTE pBuffer, UINT32 bufferLen, PS break; default: - if(!IS_STUN_PACKET(pBuffer)) { + if (!IS_STUN_PACKET(pBuffer)) { CHK_STATUS(hexEncode(pBuffer, bufferLen, NULL, &hexStrLen)); hexStr = MEMCALLOC(1, hexStrLen * SIZEOF(CHAR)); CHK(hexStr != NULL, STATUS_NOT_ENOUGH_MEMORY); CHK_STATUS(hexEncode(pBuffer, bufferLen, hexStr, &hexStrLen)); DLOGW("Dropping unrecognized STUN packet. Packet type: 0x%02x. Packet content: \n\t%s", stunPacketType, hexStr); SAFE_MEMFREE(hexStr); - } - else if(STUN_PACKET_IS_TYPE_ERROR(pBuffer)) { + } else if (STUN_PACKET_IS_TYPE_ERROR(pBuffer)) { CHK_STATUS(hexEncode(pBuffer, bufferLen, NULL, &hexStrLen)); hexStr = MEMCALLOC(1, hexStrLen * SIZEOF(CHAR)); CHK(hexStr != NULL, STATUS_NOT_ENOUGH_MEMORY);