Skip to content

Commit 03c85cb

Browse files
jdelaplajdelapla
authored
No more detaching threads in tests (#1889)
* No more detaching threads in tests * Missed joining in one test * lock around adding thread to vector * terminate adding threads flag * reset bool in tests * Change on Ice handler after done with test to stop accessing out of scope variables after scope is closed * unused params
1 parent b82923c commit 03c85cb

File tree

3 files changed

+166
-45
lines changed

3 files changed

+166
-45
lines changed

‎tst/PeerConnectionFunctionalityTest.cpp

+79-25
Original file line numberDiff line numberDiff line change
@@ -60,27 +60,43 @@ TEST_F(PeerConnectionFunctionalityTest, connectTwoPeersWithDelay)
6060
RtcSessionDescriptionInit sdp;
6161
SIZE_T connectedCount = 0;
6262
PRtcPeerConnection offerPc = NULL, answerPc = NULL;
63+
PeerContainer offer;
64+
PeerContainer answer;
6365

6466
MEMSET(&configuration, 0x00, SIZEOF(RtcConfiguration));
6567

6668
EXPECT_EQ(createPeerConnection(&configuration, &offerPc), STATUS_SUCCESS);
6769
EXPECT_EQ(createPeerConnection(&configuration, &answerPc), STATUS_SUCCESS);
6870

6971
auto onICECandidateHdlr = [](UINT64 customData, PCHAR candidateStr) -> void {
72+
PPeerContainer container = (PPeerContainer)customData;
7073
if (candidateStr != NULL) {
71-
std::thread(
72-
[customData](std::string candidate) {
73-
RtcIceCandidateInit iceCandidate;
74-
EXPECT_EQ(STATUS_SUCCESS, deserializeRtcIceCandidateInit((PCHAR) candidate.c_str(), STRLEN(candidate.c_str()), &iceCandidate));
75-
EXPECT_EQ(STATUS_SUCCESS, addIceCandidate((PRtcPeerConnection) customData, iceCandidate.candidate));
76-
},
77-
std::string(candidateStr))
78-
.detach();
74+
container->client->lock.lock();
75+
if(!container->client->noNewThreads) {
76+
container->client->threads.push_back(std::thread(
77+
[container](std::string candidate) {
78+
RtcIceCandidateInit iceCandidate;
79+
EXPECT_EQ(STATUS_SUCCESS, deserializeRtcIceCandidateInit((PCHAR) candidate.c_str(), STRLEN(candidate.c_str()), &iceCandidate));
80+
EXPECT_EQ(STATUS_SUCCESS, addIceCandidate((PRtcPeerConnection) container->pc, iceCandidate.candidate));
81+
},
82+
std::string(candidateStr)));
83+
}
84+
container->client->lock.unlock();
7985
}
8086
};
8187

82-
EXPECT_EQ(STATUS_SUCCESS, peerConnectionOnIceCandidate(offerPc, (UINT64) answerPc, onICECandidateHdlr));
83-
EXPECT_EQ(STATUS_SUCCESS, peerConnectionOnIceCandidate(answerPc, (UINT64) offerPc, onICECandidateHdlr));
88+
offer.pc = offerPc;
89+
offer.client = this;
90+
answer.pc = answerPc;
91+
answer.client = this;
92+
93+
auto onICECandidateHdlrDone = [](UINT64 customData, PCHAR candidateStr) -> void {
94+
UNUSED_PARAM(customData);
95+
UNUSED_PARAM(candidateStr);
96+
};
97+
98+
EXPECT_EQ(STATUS_SUCCESS, peerConnectionOnIceCandidate(offerPc, (UINT64) &answer, onICECandidateHdlr));
99+
EXPECT_EQ(STATUS_SUCCESS, peerConnectionOnIceCandidate(answerPc, (UINT64) &offer, onICECandidateHdlr));
84100

85101
auto onICEConnectionStateChangeHdlr = [](UINT64 customData, RTC_PEER_CONNECTION_STATE newState) -> void {
86102
if (newState == RTC_PEER_CONNECTION_STATE_CONNECTED) {
@@ -108,6 +124,17 @@ TEST_F(PeerConnectionFunctionalityTest, connectTwoPeersWithDelay)
108124

109125
EXPECT_EQ(2, connectedCount);
110126

127+
this->lock.lock();
128+
//join all threads before leaving
129+
for (auto& th : this->threads) th.join();
130+
131+
this->threads.clear();
132+
this->noNewThreads = TRUE;
133+
this->lock.unlock();
134+
135+
EXPECT_EQ(STATUS_SUCCESS, peerConnectionOnIceCandidate(offerPc, (UINT64) 0, onICECandidateHdlrDone));
136+
EXPECT_EQ(STATUS_SUCCESS, peerConnectionOnIceCandidate(answerPc, (UINT64) 0, onICECandidateHdlrDone));
137+
111138
closePeerConnection(offerPc);
112139
closePeerConnection(answerPc);
113140

@@ -635,6 +662,9 @@ TEST_F(PeerConnectionFunctionalityTest, noLostFramesAfterConnected)
635662
ATOMIC_BOOL seenFirstFrame = FALSE;
636663
Frame videoFrame;
637664

665+
PeerContainer offer;
666+
PeerContainer answer;
667+
638668
MEMSET(&configuration, 0x00, SIZEOF(RtcConfiguration));
639669
MEMSET(&videoFrame, 0x00, SIZEOF(Frame));
640670

@@ -654,6 +684,33 @@ TEST_F(PeerConnectionFunctionalityTest, noLostFramesAfterConnected)
654684
addTrackToPeerConnection(offerPc, &offerVideoTrack, &offerVideoTransceiver, RTC_CODEC_VP8, MEDIA_STREAM_TRACK_KIND_VIDEO);
655685
addTrackToPeerConnection(answerPc, &answerVideoTrack, &answerVideoTransceiver, RTC_CODEC_VP8, MEDIA_STREAM_TRACK_KIND_VIDEO);
656686

687+
auto onICECandidateHdlr = [](UINT64 customData, PCHAR candidateStr) -> void {
688+
PPeerContainer container = (PPeerContainer)customData;
689+
if (candidateStr != NULL) {
690+
container->client->lock.lock();
691+
if(!container->client->noNewThreads) {
692+
container->client->threads.push_back(std::thread(
693+
[container](std::string candidate) {
694+
RtcIceCandidateInit iceCandidate;
695+
EXPECT_EQ(STATUS_SUCCESS, deserializeRtcIceCandidateInit((PCHAR) candidate.c_str(), STRLEN(candidate.c_str()), &iceCandidate));
696+
EXPECT_EQ(STATUS_SUCCESS, addIceCandidate((PRtcPeerConnection) container->pc, iceCandidate.candidate));
697+
},
698+
std::string(candidateStr)));
699+
}
700+
container->client->lock.unlock();
701+
}
702+
};
703+
704+
offer.pc = offerPc;
705+
offer.client = this;
706+
answer.pc = answerPc;
707+
answer.client = this;
708+
709+
auto onICECandidateHdlrDone = [](UINT64 customData, PCHAR candidateStr) -> void {
710+
UNUSED_PARAM(customData);
711+
UNUSED_PARAM(candidateStr);
712+
};
713+
657714
auto onFrameHandler = [](UINT64 customData, PFrame pFrame) -> void {
658715
UNUSED_PARAM(pFrame);
659716
if (pFrame->frameData[0] == 1) {
@@ -662,21 +719,8 @@ TEST_F(PeerConnectionFunctionalityTest, noLostFramesAfterConnected)
662719
};
663720
EXPECT_EQ(transceiverOnFrame(answerVideoTransceiver, (UINT64) &seenFirstFrame, onFrameHandler), STATUS_SUCCESS);
664721

665-
auto onICECandidateHdlr = [](UINT64 customData, PCHAR candidateStr) -> void {
666-
if (candidateStr != NULL) {
667-
std::thread(
668-
[customData](std::string candidate) {
669-
RtcIceCandidateInit iceCandidate;
670-
EXPECT_EQ(STATUS_SUCCESS, deserializeRtcIceCandidateInit((PCHAR) candidate.c_str(), STRLEN(candidate.c_str()), &iceCandidate));
671-
EXPECT_EQ(STATUS_SUCCESS, addIceCandidate((PRtcPeerConnection) customData, iceCandidate.candidate));
672-
},
673-
std::string(candidateStr))
674-
.detach();
675-
}
676-
};
677-
678-
EXPECT_EQ(STATUS_SUCCESS, peerConnectionOnIceCandidate(offerPc, (UINT64) answerPc, onICECandidateHdlr));
679-
EXPECT_EQ(STATUS_SUCCESS, peerConnectionOnIceCandidate(answerPc, (UINT64) offerPc, onICECandidateHdlr));
722+
EXPECT_EQ(STATUS_SUCCESS, peerConnectionOnIceCandidate(offerPc, (UINT64) &answer, onICECandidateHdlr));
723+
EXPECT_EQ(STATUS_SUCCESS, peerConnectionOnIceCandidate(answerPc, (UINT64) &offer, onICECandidateHdlr));
680724

681725
auto onICEConnectionStateChangeHdlr = [](UINT64 customData, RTC_PEER_CONNECTION_STATE newState) -> void {
682726
Context* pContext = (Context*) customData;
@@ -714,6 +758,16 @@ TEST_F(PeerConnectionFunctionalityTest, noLostFramesAfterConnected)
714758
THREAD_SLEEP(HUNDREDS_OF_NANOS_IN_A_MILLISECOND);
715759
}
716760

761+
this->lock.lock();
762+
for (auto& th : this->threads) th.join();
763+
764+
this->threads.clear();
765+
this->noNewThreads = TRUE;
766+
this->lock.unlock();
767+
768+
EXPECT_EQ(STATUS_SUCCESS, peerConnectionOnIceCandidate(offerPc, (UINT64) 0, onICECandidateHdlrDone));
769+
EXPECT_EQ(STATUS_SUCCESS, peerConnectionOnIceCandidate(answerPc, (UINT64) 0, onICECandidateHdlrDone));
770+
717771
MEMFREE(videoFrame.frameData);
718772
closePeerConnection(offerPc);
719773
closePeerConnection(answerPc);

‎tst/WebRTCClientTestFixture.cpp

+79-20
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,7 @@ void WebRtcClientTestBase::SetUp()
8888
mDroppedFrameIndex = 0;
8989
mExpectedFrameCount = 0;
9090
mExpectedDroppedFrameCount = 0;
91+
noNewThreads = FALSE;
9192

9293
SET_INSTRUMENTED_ALLOCATORS();
9394

@@ -218,22 +219,40 @@ bool WebRtcClientTestBase::connectTwoPeers(PRtcPeerConnection offerPc, PRtcPeerC
218219
PCHAR pAnswerCertFingerprint)
219220
{
220221
RtcSessionDescriptionInit sdp;
222+
PeerContainer offer;
223+
PeerContainer answer;
224+
this->noNewThreads = FALSE;
221225

222226
auto onICECandidateHdlr = [](UINT64 customData, PCHAR candidateStr) -> void {
227+
PPeerContainer container = (PPeerContainer)customData;
223228
if (candidateStr != NULL) {
224-
std::thread(
225-
[customData](std::string candidate) {
226-
RtcIceCandidateInit iceCandidate;
227-
EXPECT_EQ(STATUS_SUCCESS, deserializeRtcIceCandidateInit((PCHAR) candidate.c_str(), STRLEN(candidate.c_str()), &iceCandidate));
228-
EXPECT_EQ(STATUS_SUCCESS, addIceCandidate((PRtcPeerConnection) customData, iceCandidate.candidate));
229-
},
230-
std::string(candidateStr))
231-
.detach();
229+
container->client->lock.lock();
230+
if(!container->client->noNewThreads) {
231+
container->client->threads.push_back(std::thread(
232+
[container](std::string candidate) {
233+
RtcIceCandidateInit iceCandidate;
234+
EXPECT_EQ(STATUS_SUCCESS, deserializeRtcIceCandidateInit((PCHAR) candidate.c_str(), STRLEN(candidate.c_str()), &iceCandidate));
235+
EXPECT_EQ(STATUS_SUCCESS, addIceCandidate((PRtcPeerConnection) container->pc, iceCandidate.candidate));
236+
},
237+
std::string(candidateStr)));
238+
}
239+
container->client->lock.unlock();
232240
}
241+
242+
};
243+
244+
auto onICECandidateHdlrDone = [](UINT64 customData, PCHAR candidateStr) -> void {
245+
UNUSED_PARAM(customData);
246+
UNUSED_PARAM(candidateStr);
233247
};
234248

235-
EXPECT_EQ(STATUS_SUCCESS, peerConnectionOnIceCandidate(offerPc, (UINT64) answerPc, onICECandidateHdlr));
236-
EXPECT_EQ(STATUS_SUCCESS, peerConnectionOnIceCandidate(answerPc, (UINT64) offerPc, onICECandidateHdlr));
249+
offer.pc = offerPc;
250+
offer.client = this;
251+
answer.pc = answerPc;
252+
answer.client = this;
253+
254+
EXPECT_EQ(STATUS_SUCCESS, peerConnectionOnIceCandidate(offerPc, (UINT64) &answer, onICECandidateHdlr));
255+
EXPECT_EQ(STATUS_SUCCESS, peerConnectionOnIceCandidate(answerPc, (UINT64) &offer, onICECandidateHdlr));
237256

238257
auto onICEConnectionStateChangeHdlr = [](UINT64 customData, RTC_PEER_CONNECTION_STATE newState) -> void {
239258
ATOMIC_INCREMENT((PSIZE_T) customData + newState);
@@ -263,29 +282,58 @@ bool WebRtcClientTestBase::connectTwoPeers(PRtcPeerConnection offerPc, PRtcPeerC
263282
THREAD_SLEEP(HUNDREDS_OF_NANOS_IN_A_SECOND);
264283
}
265284

285+
this->lock.lock();
286+
//join all threads before leaving
287+
for (auto& th : this->threads) th.join();
288+
289+
this->threads.clear();
290+
this->noNewThreads = TRUE;
291+
this->lock.unlock();
292+
293+
EXPECT_EQ(STATUS_SUCCESS, peerConnectionOnIceCandidate(offerPc, (UINT64) 0, onICECandidateHdlrDone));
294+
EXPECT_EQ(STATUS_SUCCESS, peerConnectionOnIceCandidate(answerPc, (UINT64) 0, onICECandidateHdlrDone));
295+
296+
266297
return ATOMIC_LOAD(&this->stateChangeCount[RTC_PEER_CONNECTION_STATE_CONNECTED]) == 2;
267298
}
268299

269300
bool WebRtcClientTestBase::connectTwoPeersAsyncIce(PRtcPeerConnection offerPc, PRtcPeerConnection answerPc, PCHAR pOfferCertFingerprint,
270301
PCHAR pAnswerCertFingerprint)
271302
{
272303
RtcSessionDescriptionInit sdp;
304+
PeerContainer offer;
305+
PeerContainer answer;
306+
this->noNewThreads = FALSE;
273307

274308
auto onICECandidateHdlr = [](UINT64 customData, PCHAR candidateStr) -> void {
309+
PPeerContainer container = (PPeerContainer)customData;
275310
if (candidateStr != NULL) {
276-
std::thread(
277-
[customData](std::string candidate) {
278-
RtcIceCandidateInit iceCandidate;
279-
EXPECT_EQ(STATUS_SUCCESS, deserializeRtcIceCandidateInit((PCHAR) candidate.c_str(), STRLEN(candidate.c_str()), &iceCandidate));
280-
EXPECT_EQ(STATUS_SUCCESS, addIceCandidate((PRtcPeerConnection) customData, iceCandidate.candidate));
281-
},
282-
std::string(candidateStr))
283-
.detach();
311+
container->client->lock.lock();
312+
if(!container->client->noNewThreads) {
313+
container->client->threads.push_back(std::thread(
314+
[container](std::string candidate) {
315+
RtcIceCandidateInit iceCandidate;
316+
EXPECT_EQ(STATUS_SUCCESS, deserializeRtcIceCandidateInit((PCHAR) candidate.c_str(), STRLEN(candidate.c_str()), &iceCandidate));
317+
EXPECT_EQ(STATUS_SUCCESS, addIceCandidate((PRtcPeerConnection) container->pc, iceCandidate.candidate));
318+
},
319+
std::string(candidateStr)));
320+
}
321+
container->client->lock.unlock();
284322
}
285323
};
286324

287-
EXPECT_EQ(STATUS_SUCCESS, peerConnectionOnIceCandidate(offerPc, (UINT64) answerPc, onICECandidateHdlr));
288-
EXPECT_EQ(STATUS_SUCCESS, peerConnectionOnIceCandidate(answerPc, (UINT64) offerPc, onICECandidateHdlr));
325+
auto onICECandidateHdlrDone = [](UINT64 customData, PCHAR candidateStr) -> void {
326+
UNUSED_PARAM(customData);
327+
UNUSED_PARAM(candidateStr);
328+
};
329+
330+
offer.pc = offerPc;
331+
offer.client = this;
332+
answer.pc = answerPc;
333+
answer.client = this;
334+
335+
EXPECT_EQ(STATUS_SUCCESS, peerConnectionOnIceCandidate(offerPc, (UINT64) &answer, onICECandidateHdlr));
336+
EXPECT_EQ(STATUS_SUCCESS, peerConnectionOnIceCandidate(answerPc, (UINT64) &offer, onICECandidateHdlr));
289337

290338
auto onICEConnectionStateChangeHdlr = [](UINT64 customData, RTC_PEER_CONNECTION_STATE newState) -> void {
291339
ATOMIC_INCREMENT((PSIZE_T) customData + newState);
@@ -317,6 +365,17 @@ bool WebRtcClientTestBase::connectTwoPeersAsyncIce(PRtcPeerConnection offerPc, P
317365
THREAD_SLEEP(HUNDREDS_OF_NANOS_IN_A_SECOND);
318366
}
319367

368+
this->lock.lock();
369+
//join all threads before leaving
370+
for (auto& th : this->threads) th.join();
371+
372+
this->threads.clear();
373+
this->noNewThreads = TRUE;
374+
this->lock.unlock();
375+
376+
EXPECT_EQ(STATUS_SUCCESS, peerConnectionOnIceCandidate(offerPc, (UINT64) 0, onICECandidateHdlrDone));
377+
EXPECT_EQ(STATUS_SUCCESS, peerConnectionOnIceCandidate(answerPc, (UINT64) 0, onICECandidateHdlrDone));
378+
320379
return ATOMIC_LOAD(&this->stateChangeCount[RTC_PEER_CONNECTION_STATE_CONNECTED]) == 2;
321380
}
322381

‎tst/WebRTCClientTestFixture.h

+8
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,9 @@ class WebRtcClientTestBase : public ::testing::Test {
5959
UINT32 mRtpPacketCount;
6060
UINT32 mUriCount = 0;
6161
SIGNALING_CLIENT_HANDLE mSignalingClientHandle;
62+
std::vector<std::thread> threads;
63+
std::mutex lock;
64+
BOOL noNewThreads = FALSE;
6265

6366
WebRtcClientTestBase();
6467

@@ -339,6 +342,11 @@ class WebRtcClientTestBase : public ::testing::Test {
339342
Tag mTags[3];
340343
};
341344

345+
typedef struct {
346+
PRtcPeerConnection pc;
347+
WebRtcClientTestBase* client;
348+
} PeerContainer, *PPeerContainer;
349+
342350
} // namespace webrtcclient
343351
} // namespace video
344352
} // namespace kinesis

0 commit comments

Comments
 (0)