Skip to content

Commit

Permalink
Ensure remote description does not live in memory for viewer duration (…
Browse files Browse the repository at this point in the history 
…#1915)

* Ensure remote description does not live in memory for viewer duration

* Fix memory leak
  • Loading branch information
@disa6302
disa6302 authored Feb 16, 2024
1 parent 458bacb commit e5a41a0
Show file tree
Hide file tree
Showing 3 changed files with 43 additions and 28 deletions.
29 changes: 21 additions & 8 deletions src/source/PeerConnection/PeerConnection.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -998,6 +998,11 @@ STATUS freePeerConnection(PRtcPeerConnection* ppPeerConnection)

CHK(pKvsPeerConnection != NULL, retStatus);

// This should be freed once answer is created, but unit tests might not invoke `createAnswer`
if (pKvsPeerConnection->pRemoteSessionDescription != NULL) {
DLOGW("Remote description field not deallocated");
SAFE_MEMFREE(pKvsPeerConnection->pRemoteSessionDescription);
}
startTime = GETTIME();
/* Shutdown IceAgent first so there is no more incoming packets which can cause
* SCTP to be allocated again after SCTP is freed. */
Expand Down Expand Up @@ -1214,7 +1219,7 @@ STATUS peerConnectionGetLocalDescription(PRtcPeerConnection pRtcPeerConnection,
pRtcSessionDescriptionInit->type = SDP_TYPE_ANSWER;
}

CHK_STATUS(populateSessionDescription(pKvsPeerConnection, &(pKvsPeerConnection->remoteSessionDescription), pSessionDescription));
CHK_STATUS(populateSessionDescription(pKvsPeerConnection, pKvsPeerConnection->pRemoteSessionDescription, pSessionDescription));
CHK_STATUS(serializeSessionDescription(pSessionDescription, NULL, &serializeLen));
CHK(serializeLen <= MAX_SESSION_DESCRIPTION_INIT_SDP_LEN, STATUS_NOT_ENOUGH_MEMORY);

Expand All @@ -1238,11 +1243,11 @@ STATUS peerConnectionGetCurrentLocalDescription(PRtcPeerConnection pRtcPeerConne

CHK(pRtcPeerConnection != NULL && pRtcSessionDescriptionInit != NULL, STATUS_NULL_ARG);
// do nothing if remote session description hasn't been received
CHK(pKvsPeerConnection->remoteSessionDescription.sessionName[0] != '\0', retStatus);
CHK(pKvsPeerConnection->pRemoteSessionDescription != NULL, retStatus);

CHK(NULL != (pSessionDescription = (PSessionDescription) MEMCALLOC(1, SIZEOF(SessionDescription))), STATUS_NOT_ENOUGH_MEMORY);

CHK_STATUS(populateSessionDescription(pKvsPeerConnection, &(pKvsPeerConnection->remoteSessionDescription), pSessionDescription));
CHK_STATUS(populateSessionDescription(pKvsPeerConnection, pKvsPeerConnection->pRemoteSessionDescription, pSessionDescription));

CHK_STATUS(serializeSessionDescription(pSessionDescription, NULL, &serializeLen));
CHK(serializeLen <= MAX_SESSION_DESCRIPTION_INIT_SDP_LEN, STATUS_NOT_ENOUGH_MEMORY);
Expand Down Expand Up @@ -1279,14 +1284,15 @@ STATUS setRemoteDescription(PRtcPeerConnection pPeerConnection, PRtcSessionDescr
STATUS retStatus = STATUS_SUCCESS;
PCHAR remoteIceUfrag = NULL, remoteIcePwd = NULL;
UINT32 i, j;
PSessionDescription pSessionDescription;

CHK(pPeerConnection != NULL, STATUS_NULL_ARG);
PKvsPeerConnection pKvsPeerConnection = (PKvsPeerConnection) pPeerConnection;
PSessionDescription pSessionDescription = &pKvsPeerConnection->remoteSessionDescription;

pKvsPeerConnection->pRemoteSessionDescription = (PSessionDescription) MEMCALLOC(1, SIZEOF(SessionDescription));
pSessionDescription = pKvsPeerConnection->pRemoteSessionDescription;
CHK(pSessionDescriptionInit != NULL, STATUS_NULL_ARG);

MEMSET(pSessionDescription, 0x00, SIZEOF(SessionDescription));
pKvsPeerConnection->dtlsIsServer = FALSE;
/* Assume cant trickle at first */
NULLABLE_SET_VALUE(pKvsPeerConnection->canTrickleIce, FALSE);
Expand Down Expand Up @@ -1373,8 +1379,11 @@ STATUS setRemoteDescription(PRtcPeerConnection pPeerConnection, PRtcSessionDescr
DLOGD("REMOTE_SDP:%s\n", pSessionDescriptionInit->sdp);
}

// Remove remote description once required parameters are set from answer
if (pKvsPeerConnection->isOffer) {
SAFE_MEMFREE(pKvsPeerConnection->pRemoteSessionDescription);
}
CleanUp:

LEAVES();
return retStatus;
}
Expand Down Expand Up @@ -1403,7 +1412,7 @@ STATUS createOffer(PRtcPeerConnection pPeerConnection, PRtcSessionDescriptionIni
#endif

CHK_STATUS(setPayloadTypesForOffer(pKvsPeerConnection->pCodecTable));
CHK_STATUS(populateSessionDescription(pKvsPeerConnection, &(pKvsPeerConnection->remoteSessionDescription), pSessionDescription));
CHK_STATUS(populateSessionDescription(pKvsPeerConnection, pKvsPeerConnection->pRemoteSessionDescription, pSessionDescription));
CHK_STATUS(serializeSessionDescription(pSessionDescription, NULL, &serializeLen));
CHK(serializeLen <= MAX_SESSION_DESCRIPTION_INIT_SDP_LEN, STATUS_NOT_ENOUGH_MEMORY);
CHK_STATUS(serializeSessionDescription(pSessionDescription, pSessionDescriptionInit->sdp, &serializeLen));
Expand All @@ -1427,7 +1436,7 @@ STATUS createAnswer(PRtcPeerConnection pPeerConnection, PRtcSessionDescriptionIn
PKvsPeerConnection pKvsPeerConnection = (PKvsPeerConnection) pPeerConnection;

CHK(pKvsPeerConnection != NULL && pSessionDescriptionInit != NULL, STATUS_NULL_ARG);
CHK(pKvsPeerConnection->remoteSessionDescription.sessionName[0] != '\0', STATUS_PEERCONNECTION_CREATE_ANSWER_WITHOUT_REMOTE_DESCRIPTION);
CHK(pKvsPeerConnection->pRemoteSessionDescription != NULL, STATUS_PEERCONNECTION_CREATE_ANSWER_WITHOUT_REMOTE_DESCRIPTION);

pSessionDescriptionInit->type = SDP_TYPE_ANSWER;
pKvsPeerConnection->isOffer = FALSE;
Expand All @@ -1437,6 +1446,10 @@ STATUS createAnswer(PRtcPeerConnection pPeerConnection, PRtcSessionDescriptionIn
if (NULL != GETENV(DEBUG_LOG_SDP)) {
DLOGD("LOCAL_SDP:%s", pSessionDescriptionInit->sdp);
}

// Once answer is created, remote SDP is not needed anymore. We also clear only if
// answer SDP is successfully created
SAFE_MEMFREE(pKvsPeerConnection->pRemoteSessionDescription);
CleanUp:

LEAVES();
Expand Down
2 changes: 1 addition & 1 deletion src/source/PeerConnection/PeerConnection.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ typedef struct {

PSctpSession pSctpSession;

SessionDescription remoteSessionDescription;
PSessionDescription pRemoteSessionDescription;
PDoubleList pTransceivers;
PDoubleList pFakeTransceivers;
PDoubleList pAnswerTransceivers;
Expand Down
40 changes: 21 additions & 19 deletions src/source/PeerConnection/SessionDescription.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -413,9 +413,10 @@ STATUS populateSingleMediaSection(PKvsPeerConnection pKvsPeerConnection, PKvsRtp
CHK_STATUS(hashTableGet(pUnknownCodecPayloadTypesTable, unknownCodecHashTableKey, &payloadType));
} else {
CHK_STATUS(hashTableGet(pKvsPeerConnection->pCodecTable, pRtcMediaStreamTrack->codec, &payloadType));
currentFmtp = fmtpForPayloadType(payloadType, &(pKvsPeerConnection->remoteSessionDescription));
if (!pKvsPeerConnection->isOffer) {
currentFmtp = fmtpForPayloadType(payloadType, pRemoteSessionDescription);
}
}

if (pRtcMediaStreamTrack->kind == MEDIA_STREAM_TRACK_KIND_VIDEO) {
if (pRtcMediaStreamTrack->codec == RTC_CODEC_H264_PROFILE_42E01F_LEVEL_ASYMMETRY_ALLOWED_PACKETIZATION_MODE) {
retStatus = hashTableGet(pKvsPeerConnection->pRtxTable, RTC_RTX_CODEC_H264_PROFILE_42E01F_LEVEL_ASYMMETRY_ALLOWED_PACKETIZATION_MODE,
Expand Down Expand Up @@ -555,11 +556,14 @@ STATUS populateSingleMediaSection(PKvsPeerConnection pKvsPeerConnection, PKvsRtp
attributeCount++;

STRCPY(pSdpMediaDescription->sdpAttributes[attributeCount].attributeName, "mid");
// check all session attribute lines to see if a line with mid is present. If it is present, copy its content and break
for (i = 0; i < pRemoteSessionDescription->mediaDescriptions[mediaSectionId].mediaAttributesCount; i++) {
if (STRCMP(pRemoteSessionDescription->mediaDescriptions[mediaSectionId].sdpAttributes[i].attributeName, MID_KEY) == 0) {
STRCPY(remoteSdpAttributeValue, pRemoteSessionDescription->mediaDescriptions[mediaSectionId].sdpAttributes[i].attributeValue);
break;

if (!pKvsPeerConnection->isOffer) {
// check all session attribute lines to see if a line with mid is present. If it is present, copy its content and break
for (i = 0; i < pRemoteSessionDescription->mediaDescriptions[mediaSectionId].mediaAttributesCount; i++) {
if (STRCMP(pRemoteSessionDescription->mediaDescriptions[mediaSectionId].sdpAttributes[i].attributeName, MID_KEY) == 0) {
STRCPY(remoteSdpAttributeValue, pRemoteSessionDescription->mediaDescriptions[mediaSectionId].sdpAttributes[i].attributeValue);
break;
}
}
}

Expand Down Expand Up @@ -620,7 +624,6 @@ STATUS populateSingleMediaSection(PKvsPeerConnection pKvsPeerConnection, PKvsRtp

STRCPY(pSdpMediaDescription->sdpAttributes[attributeCount].attributeName, "rtcp-mux");
attributeCount++;

if (mediaSectionId != 0) {
STRCPY(pSdpMediaDescription->sdpAttributes[attributeCount].attributeName, "rtcp-rsize");
attributeCount++;
Expand Down Expand Up @@ -889,18 +892,15 @@ STATUS populateSessionDescriptionMedia(PKvsPeerConnection pKvsPeerConnection, PS
UINT32 unknownHashTableBucketCount = 0;

CHK_STATUS(dtlsSessionGetLocalCertificateFingerprint(pKvsPeerConnection->pDtlsSession, certificateFingerprint, CERTIFICATE_FINGERPRINT_LENGTH));

if (pKvsPeerConnection->isOffer) {
pDtlsRole = DTLS_ROLE_ACTPASS;

CHK_STATUS(doubleListGetHeadNode(pKvsPeerConnection->pTransceivers, &pCurNode));
while (pCurNode != NULL) {
CHK_STATUS(doubleListGetNodeData(pCurNode, &data));
pCurNode = pCurNode->pNext;
pKvsRtpTransceiver = (PKvsRtpTransceiver) data;
if (pKvsRtpTransceiver != NULL) {
CHK(pLocalSessionDescription->mediaCount < MAX_SDP_SESSION_MEDIA_COUNT, STATUS_SESSION_DESCRIPTION_MAX_MEDIA_COUNT);

// If generating answer, need to check if Local Description is present in remote -- if not, we don't need to create a local
// description for it or else our Answer will have an extra m-line, for offer the local is the offer itself, don't care about remote
CHK_STATUS(populateSingleMediaSection(
Expand All @@ -922,7 +922,6 @@ STATUS populateSessionDescriptionMedia(PKvsPeerConnection pKvsPeerConnection, PS
// if an m-line does not have a corresponding transceiver created by the user, we create a fake transceiver
CHK_STATUS(findTransceiversByRemoteDescription(pKvsPeerConnection, pRemoteSessionDescription, pUnknownCodecPayloadTypesTable,
pUnknownCodecRtpmapTable));

// pAnswerTransceivers contains transceivers created by the user as well as fake transceivers
CHK_STATUS(doubleListGetHeadNode(pKvsPeerConnection->pAnswerTransceivers, &pCurNode));
while (pCurNode != NULL) {
Expand Down Expand Up @@ -989,12 +988,14 @@ STATUS populateSessionDescription(PKvsPeerConnection pKvsPeerConnection, PSessio
UINT32 i, sizeRemaining;
INT32 charsCopied;

CHK(pKvsPeerConnection != NULL && pLocalSessionDescription != NULL && pRemoteSessionDescription != NULL, STATUS_NULL_ARG);
CHK(pKvsPeerConnection != NULL && pLocalSessionDescription != NULL, STATUS_NULL_ARG);
if (!pKvsPeerConnection->isOffer) {
CHK(pRemoteSessionDescription != NULL, STATUS_NULL_ARG);
}
CHK_STATUS(populateSessionDescriptionMedia(pKvsPeerConnection, pRemoteSessionDescription, pLocalSessionDescription));
MEMSET(bundleValue, 0, MAX_SDP_ATTRIBUTE_VALUE_LENGTH);
MEMSET(wmsValue, 0, MAX_SDP_ATTRIBUTE_VALUE_LENGTH);
MEMSET(remoteSdpAttributeValue, 0, MAX_SDP_ATTRIBUTE_VALUE_LENGTH);

STRCPY(pLocalSessionDescription->sdpOrigin.userName, "-");
pLocalSessionDescription->sdpOrigin.sessionId = RAND();
pLocalSessionDescription->sdpOrigin.sessionVersion = 2;
Expand All @@ -1011,18 +1012,19 @@ STATUS populateSessionDescription(PKvsPeerConnection pKvsPeerConnection, PSessio
STRCPY(pLocalSessionDescription->sdpAttributes[0].attributeName, "group");
STRCPY(pLocalSessionDescription->sdpAttributes[0].attributeValue, BUNDLE_KEY);
pLocalSessionDescription->sessionAttributesCount++;

if (pKvsPeerConnection->canTrickleIce.value) {
STRCPY(pLocalSessionDescription->sdpAttributes[pLocalSessionDescription->sessionAttributesCount].attributeName, "ice-options");
STRCPY(pLocalSessionDescription->sdpAttributes[pLocalSessionDescription->sessionAttributesCount].attributeValue, "trickle");
pLocalSessionDescription->sessionAttributesCount++;
}

// check all session attribute lines to see if a line with BUNDLE is present. If it is present, copy its content and break
for (i = 0; i < pRemoteSessionDescription->sessionAttributesCount; i++) {
if (STRSTR(pRemoteSessionDescription->sdpAttributes[i].attributeValue, BUNDLE_KEY) != NULL) {
STRCPY(remoteSdpAttributeValue, pRemoteSessionDescription->sdpAttributes[i].attributeValue + ARRAY_SIZE(BUNDLE_KEY) - 1);
break;
if (!pKvsPeerConnection->isOffer) {
for (i = 0; i < pRemoteSessionDescription->sessionAttributesCount; i++) {
if (STRSTR(pRemoteSessionDescription->sdpAttributes[i].attributeValue, BUNDLE_KEY) != NULL) {
STRCPY(remoteSdpAttributeValue, pRemoteSessionDescription->sdpAttributes[i].attributeValue + ARRAY_SIZE(BUNDLE_KEY) - 1);
break;
}
}
}

Expand Down

0 comments on commit e5a41a0

Please sign in to comment.