Stop leaking FlexfecReceiveStream objects after call shutdown.

Stop leaking FlexfecReceiveStream objects after call shutdown.

BUG=webrtc:7017
Review-Url: https://codereview.webrtc.org/2645703003
Cr-Commit-Position: refs/heads/master@{#16212}
Committed: https://chromium.googlesource.com/external/webrtc/+/9c3d4c4d881be595dd8e68e2130f23d9f4fab057

[brandtr, 2017-01-19 15:38:47]

brandtr@webrtc.org changed reviewers:
+ stefan@webrtc.org

[brandtr, 2017-01-20 13:33:25]

Description was changed from

==========
Stop leaking FlexfecReceiveStream objects after call shutdown.

BUG=webrtc:7017
==========

to

==========
Stop leaking FlexfecReceiveStream objects after call shutdown.

BUG=webrtc:7017
==========

[brandtr, 2017-01-20 13:33:25]

brandtr@webrtc.org changed reviewers:
- stefan@webrtc.org

[brandtr, 2017-01-20 13:33:38]

brandtr@webrtc.org changed reviewers:
+ stefan@webrtc.org

[brandtr, 2017-01-20 13:33:42]

Patchset #2 (id:20001) has been deleted

[brandtr, 2017-01-20 13:52:10]

Extra simple test case.

[brandtr, 2017-01-20 13:52:29]

Patchset #2 (id:40001) has been deleted

[stefan-webrtc, 2017-01-20 14:48:36]

On 2017/01/20 13:52:10, brandtr wrote:
> Extra simple test case.

Can't find the test case?

[brandtr, 2017-01-20 15:00:50]

Simplified test case.

[brandtr, 2017-01-20 15:04:50]

https://codereview.webrtc.org/2645703003/diff/60001/webrtc/media/engine/webrt...
File webrtc/media/engine/webrtcvideoengine2_unittest.cc (right):

https://codereview.webrtc.org/2645703003/diff/60001/webrtc/media/engine/webrt...
webrtc/media/engine/webrtcvideoengine2_unittest.cc:2427:
TEST_F(WebRtcVideoChannel2FlexfecTest, SetRecvCodecsWithFec) {
Here it is. This is a slightly simplified version of the test in 
https://cs.chromium.org/chromium/src/third_party/webrtc/media/engine/webrtcvi...

From the perspective of this bug, the point of this test is to exercise
~WebRtcVideoChannel2 when a FlexfecReceiveStream object has been created. I ran
it locally 10000 times with asan+lsan, but couldn't get any leak warnings.
Perhaps I didn't configure the sanitizers correctly.

The earlier test linked above should also have exercised ~WebRtcVideoChannel2,
but I haven't received any bug reports from that one either.

[stefan-webrtc, 2017-01-20 15:10:37]

kjellander, any idea why this leak wasn't caught by the asan bots?

[brandtr, 2017-01-23 07:26:15]

On 2017/01/20 15:10:37, stefan-webrtc wrote:
> kjellander, any idea why this leak wasn't caught by the asan bots?

I have a theory why the leak wasn't caught: the FakeCall object used by these
unit tests do not use dynamic memory allocation when creating the
FakeFlexfecReceiveStream, whereas the production Call object does use dynamic
memory allocation when creating the FlexfecReceiveStream object. I.e., there is
no leak in the tests, but there is a leak in the production code. See discussion
about why there is a difference between FakeCall and Call here:
https://codereview.webrtc.org/2511703002/.

Today I'll update this CL to try to trigger the leak in the tests as well.

[brandtr, 2017-01-23 08:36:11]

Harmonize FakeCall implementation with Call. Revert fix to expose leak on bots.

[brandtr, 2017-01-23 08:52:44]

Local results with FakeCall/Call harmonization and fix reverted:

 ✘  brandtr@brandtr  ~/src/webrtc/src   R-Fix_FlexfecReceiveStream_leak 
ASAN_OPTIONS="symbolize=1 detect_leaks=1" out/Asan/rtc_media_unittests
--gtest_filter="*SetRecvCodecsWithFec*"
Note: Google Test filter = *SetRecvCodecsWithFec*
[==========] Running 1 test from 1 test case.
[----------] Global test environment set-up.
[----------] 1 test from WebRtcVideoChannel2FlexfecTest
[ RUN      ] WebRtcVideoChannel2FlexfecTest.SetRecvCodecsWithFec
[000:000] (cpu_info.cc:50): Available number of cores: 12
[000:000] (bitrate_prober.cc:52): Bandwidth probing enabled, set to inactive
[000:000] (remote_bitrate_estimator_single_stream.cc:61):
RemoteBitrateEstimatorSingleStream: Instantiating.
[000:001] (congestion_controller.cc:294): SignalNetworkState Down
[000:001] (paced_sender.cc:276): PacedSender paused.
[000:002] (webrtcvideoengine2.cc:475): WebRtcVideoEngine2::WebRtcVideoEngine2()
[000:002] (webrtcvideoengine2.cc:615): Internally supported codecs:
{VideoCodec[0:VP8], VideoCodec[0:VP9], VideoCodec[0:red], VideoCodec[0:ulpfec],
VideoCodec[0:flexfec-03]}
[000:002] (webrtcvideoengine2.cc:483): WebRtcVideoEngine2::Init
[000:002] (webrtcvideoengine2.cc:492): CreateChannel. Options: VideoOptions {}
[000:002] (webrtcvideoengine2.cc:615): Internally supported codecs:
{VideoCodec[0:VP8], VideoCodec[0:VP9], VideoCodec[0:red], VideoCodec[0:ulpfec],
VideoCodec[0:flexfec-03]}
[000:003] (webrtcvideoengine2.cc:615): Internally supported codecs:
{VideoCodec[0:VP8], VideoCodec[0:VP9], VideoCodec[0:red], VideoCodec[0:ulpfec],
VideoCodec[0:flexfec-03]}
[000:003] (webrtcvideoengine2.cc:615): Internally supported codecs:
{VideoCodec[0:VP8], VideoCodec[0:VP9], VideoCodec[0:red], VideoCodec[0:ulpfec],
VideoCodec[0:flexfec-03]}
[000:003] (webrtcvideoengine2.cc:760): SetSendParameters: {codecs:
[VideoCodec[96:VP8], VideoCodec[97:rtx], VideoCodec[98:VP9], VideoCodec[99:rtx],
VideoCodec[100:red], VideoCodec[101:rtx], VideoCodec[102:ulpfec],
VideoCodec[103:flexfec-03]], extensions: [], max_bandwidth_bps: -1, }
[000:003] (webrtcvideoengine2.cc:615): Internally supported codecs:
{VideoCodec[0:VP8], VideoCodec[0:VP9], VideoCodec[0:red], VideoCodec[0:ulpfec],
VideoCodec[0:flexfec-03]}
[000:003] (webrtcvideoengine2.cc:769): Using codec: VideoCodec[96:VP8]
[000:003] (webrtcvideoengine2.cc:808): SetFeedbackOptions on all the receive
streams because the send codec or RTCP mode has changed.
[000:003] (webrtcvideoengine2.cc:1160): AddRecvStream:
{ssrcs:[1];ssrc_groups:{semantics:FEC-FR;ssrcs:[1,5]};cname:cname;}
[000:003] (webrtcvideoengine2.cc:615): Internally supported codecs:
{VideoCodec[0:VP8], VideoCodec[0:VP9], VideoCodec[0:red], VideoCodec[0:ulpfec],
VideoCodec[0:flexfec-03]}
[000:003] (webrtcvideoengine2.cc:615): Internally supported codecs:
{VideoCodec[0:VP8], VideoCodec[0:VP9], VideoCodec[0:red], VideoCodec[0:ulpfec],
VideoCodec[0:flexfec-03]}
[000:003] (webrtcvideoengine2.cc:953): SetRecvParameters: {codecs:
[VideoCodec[96:VP8], VideoCodec[103:flexfec-03]], extensions: []}
[000:003] (webrtcvideoengine2.cc:615): Internally supported codecs:
{VideoCodec[0:VP8], VideoCodec[0:VP9], VideoCodec[0:red], VideoCodec[0:ulpfec],
VideoCodec[0:flexfec-03]}
[000:003] (webrtcvideoengine2.cc:962): Changing recv codecs from
{VideoCodec[96:VP8], VideoCodec[98:VP9]} to {VideoCodec[96:VP8]}
[000:003] (webrtcvideoengine2.cc:2271): RecreateWebRtcStream (recv) because of
SetRecvParameters
[000:003] (webrtcvideoengine2.cc:615): Internally supported codecs:
{VideoCodec[0:VP8], VideoCodec[0:VP9], VideoCodec[0:red], VideoCodec[0:ulpfec],
VideoCodec[0:flexfec-03]}
[000:003] (webrtcvideoengine2.cc:479): WebRtcVideoEngine2::~WebRtcVideoEngine2
[       OK ] WebRtcVideoChannel2FlexfecTest.SetRecvCodecsWithFec (4 ms)
[----------] 1 test from WebRtcVideoChannel2FlexfecTest (4 ms total)

[----------] Global test environment tear-down
[==========] 1 test from 1 test case ran. (4 ms total)
[  PASSED  ] 1 test.

=================================================================
==9311==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 96 byte(s) in 1 object(s) allocated from:
    #0 0x51f36b in operator new(unsigned long)
(/usr/local/google/home/brandtr/src/webrtc/src/out/Asan/rtc_media_unittests+0x51f36b)
    #1 0xafdc01 in
cricket::FakeCall::CreateFlexfecReceiveStream(webrtc::FlexfecReceiveStream::Config
const&) webrtc/media/engine/fakewebrtccall.cc:489:43
    #2 0xcbb6f8 in
cricket::WebRtcVideoChannel2::WebRtcVideoReceiveStream::RecreateWebRtcStream()
webrtc/media/engine/webrtcvideoengine2.cc:2288:30
    #3 0xca5823 in
cricket::WebRtcVideoChannel2::WebRtcVideoReceiveStream::SetRecvParameters(cricket::WebRtcVideoChannel2::ChangedRecvParameters
const&) webrtc/media/engine/webrtcvideoengine2.cc:2272:5
    #4 0xca4a02 in
cricket::WebRtcVideoChannel2::SetRecvParameters(cricket::VideoRecvParameters
const&) webrtc/media/engine/webrtcvideoengine2.cc:971:18
    #5 0x77d336 in
cricket::WebRtcVideoChannel2FlexfecTest_SetRecvCodecsWithFec_Test::TestBody()
webrtc/media/engine/webrtcvideoengine2_unittest.cc:2435:3
    #6 0x1586267 in HandleExceptionsInMethodIfSupported<testing::Test, void>
testing/gtest/src/gtest.cc:2458:12
    #7 0x1586267 in testing::Test::Run() testing/gtest/src/gtest.cc:2474
    #8 0x1587444 in testing::TestInfo::Run() testing/gtest/src/gtest.cc:2656:11
    #9 0x1588556 in testing::TestCase::Run() testing/gtest/src/gtest.cc:2774:28
    #10 0x159c326 in testing::internal::UnitTestImpl::RunAllTests()
testing/gtest/src/gtest.cc:4647:43
    #11 0x159b845 in
HandleExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool>
testing/gtest/src/gtest.cc:2458:12
    #12 0x159b845 in testing::UnitTest::Run() testing/gtest/src/gtest.cc:4255
    #13 0xc6a8bc in RUN_ALL_TESTS testing/gtest/include/gtest/gtest.h:2237:46
    #14 0xc6a8bc in main webrtc/base/unittest_main.cc:109
    #15 0x7f588c07bf44 in __libc_start_main
/build/eglibc-oGUzwX/eglibc-2.19/csu/libc-start.c:287

Indirect leak of 4 byte(s) in 1 object(s) allocated from:
    #0 0x51f36b in operator new(unsigned long)
(/usr/local/google/home/brandtr/src/webrtc/src/out/Asan/rtc_media_unittests+0x51f36b)
    #1 0x58bcc7 in __allocate
buildtools/third_party/libc++/trunk/include/new:168:10
    #2 0x58bcc7 in allocate
buildtools/third_party/libc++/trunk/include/memory:1729
    #3 0x58bcc7 in allocate
buildtools/third_party/libc++/trunk/include/memory:1488
    #4 0x58bcc7 in allocate
buildtools/third_party/libc++/trunk/include/vector:930
    #5 0x58bcc7 in std::__1::vector<unsigned int, std::__1::allocator<unsigned
int> >::vector(std::__1::vector<unsigned int, std::__1::allocator<unsigned int>
> const&) buildtools/third_party/libc++/trunk/include/vector:1204
    #6 0xafdc62 in Config webrtc/call/flexfec_receive_stream.h:33:10
    #7 0xafdc62 in FakeFlexfecReceiveStream
webrtc/media/engine/fakewebrtccall.cc:309
    #8 0xafdc62 in
cricket::FakeCall::CreateFlexfecReceiveStream(webrtc::FlexfecReceiveStream::Config
const&) webrtc/media/engine/fakewebrtccall.cc:489
    #9 0xcbb6f8 in
cricket::WebRtcVideoChannel2::WebRtcVideoReceiveStream::RecreateWebRtcStream()
webrtc/media/engine/webrtcvideoengine2.cc:2288:30
    #10 0xca5823 in
cricket::WebRtcVideoChannel2::WebRtcVideoReceiveStream::SetRecvParameters(cricket::WebRtcVideoChannel2::ChangedRecvParameters
const&) webrtc/media/engine/webrtcvideoengine2.cc:2272:5
    #11 0xca4a02 in
cricket::WebRtcVideoChannel2::SetRecvParameters(cricket::VideoRecvParameters
const&) webrtc/media/engine/webrtcvideoengine2.cc:971:18
    #12 0x77d336 in
cricket::WebRtcVideoChannel2FlexfecTest_SetRecvCodecsWithFec_Test::TestBody()
webrtc/media/engine/webrtcvideoengine2_unittest.cc:2435:3
    #13 0x1586267 in HandleExceptionsInMethodIfSupported<testing::Test, void>
testing/gtest/src/gtest.cc:2458:12
    #14 0x1586267 in testing::Test::Run() testing/gtest/src/gtest.cc:2474
    #15 0x1587444 in testing::TestInfo::Run() testing/gtest/src/gtest.cc:2656:11
    #16 0x1588556 in testing::TestCase::Run() testing/gtest/src/gtest.cc:2774:28
    #17 0x159c326 in testing::internal::UnitTestImpl::RunAllTests()
testing/gtest/src/gtest.cc:4647:43
    #18 0x159b845 in
HandleExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool>
testing/gtest/src/gtest.cc:2458:12
    #19 0x159b845 in testing::UnitTest::Run() testing/gtest/src/gtest.cc:4255
    #20 0xc6a8bc in RUN_ALL_TESTS testing/gtest/include/gtest/gtest.h:2237:46
    #21 0xc6a8bc in main webrtc/base/unittest_main.cc:109
    #22 0x7f588c07bf44 in __libc_start_main
/build/eglibc-oGUzwX/eglibc-2.19/csu/libc-start.c:287

SUMMARY: AddressSanitizer: 100 byte(s) leaked in 2 allocation(s).

[brandtr, 2017-01-23 08:53:08]

Apply fix again.

[brandtr, 2017-01-23 08:53:46]

Local results with fix reapplied:

 brandtr@brandtr  ~/src/webrtc/src   R-Fix_FlexfecReceiveStream_leak 
ASAN_OPTIONS="symbolize=1 detect_leaks=1" out/Asan/rtc_media_unittests
--gtest_filter="*SetRecvCodecsWithFec*"
Note: Google Test filter = *SetRecvCodecsWithFec*
[==========] Running 1 test from 1 test case.
[----------] Global test environment set-up.
[----------] 1 test from WebRtcVideoChannel2FlexfecTest
[ RUN      ] WebRtcVideoChannel2FlexfecTest.SetRecvCodecsWithFec
[000:000] (cpu_info.cc:50): Available number of cores: 12
[000:001] (bitrate_prober.cc:52): Bandwidth probing enabled, set to inactive
[000:001] (remote_bitrate_estimator_single_stream.cc:61):
RemoteBitrateEstimatorSingleStream: Instantiating.
[000:001] (congestion_controller.cc:294): SignalNetworkState Down
[000:001] (paced_sender.cc:276): PacedSender paused.
[000:006] (webrtcvideoengine2.cc:475): WebRtcVideoEngine2::WebRtcVideoEngine2()
[000:006] (webrtcvideoengine2.cc:615): Internally supported codecs:
{VideoCodec[0:VP8], VideoCodec[0:VP9], VideoCodec[0:red], VideoCodec[0:ulpfec],
VideoCodec[0:flexfec-03]}
[000:006] (webrtcvideoengine2.cc:483): WebRtcVideoEngine2::Init
[000:006] (webrtcvideoengine2.cc:492): CreateChannel. Options: VideoOptions {}
[000:006] (webrtcvideoengine2.cc:615): Internally supported codecs:
{VideoCodec[0:VP8], VideoCodec[0:VP9], VideoCodec[0:red], VideoCodec[0:ulpfec],
VideoCodec[0:flexfec-03]}
[000:006] (webrtcvideoengine2.cc:615): Internally supported codecs:
{VideoCodec[0:VP8], VideoCodec[0:VP9], VideoCodec[0:red], VideoCodec[0:ulpfec],
VideoCodec[0:flexfec-03]}
[000:006] (webrtcvideoengine2.cc:615): Internally supported codecs:
{VideoCodec[0:VP8], VideoCodec[0:VP9], VideoCodec[0:red], VideoCodec[0:ulpfec],
VideoCodec[0:flexfec-03]}
[000:007] (webrtcvideoengine2.cc:760): SetSendParameters: {codecs:
[VideoCodec[96:VP8], VideoCodec[97:rtx], VideoCodec[98:VP9], VideoCodec[99:rtx],
VideoCodec[100:red], VideoCodec[101:rtx], VideoCodec[102:ulpfec],
VideoCodec[103:flexfec-03]], extensions: [], max_bandwidth_bps: -1, }
[000:007] (webrtcvideoengine2.cc:615): Internally supported codecs:
{VideoCodec[0:VP8], VideoCodec[0:VP9], VideoCodec[0:red], VideoCodec[0:ulpfec],
VideoCodec[0:flexfec-03]}
[000:007] (webrtcvideoengine2.cc:769): Using codec: VideoCodec[96:VP8]
[000:007] (webrtcvideoengine2.cc:808): SetFeedbackOptions on all the receive
streams because the send codec or RTCP mode has changed.
[000:007] (webrtcvideoengine2.cc:1160): AddRecvStream:
{ssrcs:[1];ssrc_groups:{semantics:FEC-FR;ssrcs:[1,5]};cname:cname;}
[000:007] (webrtcvideoengine2.cc:615): Internally supported codecs:
{VideoCodec[0:VP8], VideoCodec[0:VP9], VideoCodec[0:red], VideoCodec[0:ulpfec],
VideoCodec[0:flexfec-03]}
[000:007] (webrtcvideoengine2.cc:615): Internally supported codecs:
{VideoCodec[0:VP8], VideoCodec[0:VP9], VideoCodec[0:red], VideoCodec[0:ulpfec],
VideoCodec[0:flexfec-03]}
[000:007] (webrtcvideoengine2.cc:953): SetRecvParameters: {codecs:
[VideoCodec[96:VP8], VideoCodec[103:flexfec-03]], extensions: []}
[000:007] (webrtcvideoengine2.cc:615): Internally supported codecs:
{VideoCodec[0:VP8], VideoCodec[0:VP9], VideoCodec[0:red], VideoCodec[0:ulpfec],
VideoCodec[0:flexfec-03]}
[000:007] (webrtcvideoengine2.cc:962): Changing recv codecs from
{VideoCodec[96:VP8], VideoCodec[98:VP9]} to {VideoCodec[96:VP8]}
[000:007] (webrtcvideoengine2.cc:2274): RecreateWebRtcStream (recv) because of
SetRecvParameters
[000:007] (webrtcvideoengine2.cc:615): Internally supported codecs:
{VideoCodec[0:VP8], VideoCodec[0:VP9], VideoCodec[0:red], VideoCodec[0:ulpfec],
VideoCodec[0:flexfec-03]}
[000:007] (webrtcvideoengine2.cc:479): WebRtcVideoEngine2::~WebRtcVideoEngine2
[       OK ] WebRtcVideoChannel2FlexfecTest.SetRecvCodecsWithFec (8 ms)
[----------] 1 test from WebRtcVideoChannel2FlexfecTest (8 ms total)

[----------] Global test environment tear-down
[==========] 1 test from 1 test case ran. (9 ms total)
[  PASSED  ] 1 test.

[stefan-webrtc, 2017-01-23 10:40:31]

lgtm

[brandtr, 2017-01-23 10:46:32]

Rebase.

[brandtr, 2017-01-23 11:41:48]

The CQ bit was checked by brandtr@webrtc.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-23 11:41:57]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...

[commit-bot: I haz the power, 2017-01-23 12:52:47]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-23 12:52:48]

Dry run: This issue passed the CQ dry run.

[brandtr, 2017-01-23 14:56:52]

The CQ bit was checked by brandtr@webrtc.org

[brandtr, 2017-01-23 14:56:53]

The patchset sent to the CQ was uploaded after l-g-t-m from stefan@webrtc.org
Link to the patchset: https://codereview.webrtc.org/2645703003/#ps120001 (title:
"Rebase.")

[commit-bot: I haz the power, 2017-01-23 14:57:01]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...

[commit-bot: I haz the power, 2017-01-23 14:59:13]

CQ is committing da patch.

Bot data: {"patchset_id": 120001, "attempt_start_ts": 1485183412666230,
"parent_rev": "a067013e909e3d33adcd51b5609cbcf8f25f5e7d", "commit_rev":
"9c3d4c4d881be595dd8e68e2130f23d9f4fab057"}

[commit-bot: I haz the power, 2017-01-23 14:59:18]

Description was changed from

==========
Stop leaking FlexfecReceiveStream objects after call shutdown.

BUG=webrtc:7017
==========

to

==========
Stop leaking FlexfecReceiveStream objects after call shutdown.

BUG=webrtc:7017

Review-Url: https://codereview.webrtc.org/2645703003
Cr-Commit-Position: refs/heads/master@{#16212}
Committed:
https://chromium.googlesource.com/external/webrtc/+/9c3d4c4d881be595dd8e68e21...
==========

[commit-bot: I haz the power, 2017-01-23 14:59:19]

Committed patchset #5 (id:120001) as
https://chromium.googlesource.com/external/webrtc/+/9c3d4c4d881be595dd8e68e21...