1408523 - permaleak of 288 bytes of thread stuff in dom/media/tests/mochitest/test_peerConnection_basicH264Video.html

Status: RESOLVED FIXED

(Core :: WebRTC: Signaling, defect, P2)

Description

[Andrew McCreight [:mccr8]]

This seems to affect Linux32, but not Linux64 or Windows, in mda3.

TEST-INFO | leakcheck | tab process: leaked 1 CondVar
TEST-INFO | leakcheck | tab process: leaked 1 Mutex
TEST-INFO | leakcheck | tab process: leaked 1 Runnable
TEST-INFO | leakcheck | tab process: leaked 1 ThreadEventTarget
TEST-INFO | leakcheck | tab process: leaked 1 ThreadTargetSink
TEST-INFO | leakcheck | tab process: leaked 4 nsTArray_base
TEST-INFO | leakcheck | tab process: leaked 1 nsThread

TEST-UNEXPECTED-FAIL | leakcheck | tab process: 288 bytes leaked (CondVar, Mutex, Runnable, ThreadEventTarget, ThreadTargetSink, ...)

Comment 1

[Andrew McCreight [:mccr8]]

This also affects OS X mda, at a size of 896 bytes.

Comment 2

[Andrew McCreight [:mccr8]]

I narrowed this down to test_peerConnection_basicH264Video.html.

Comment 3

[Andrew McCreight [:mccr8]]

More tests may leak, but this seems to be responsible for at least some of the leaks.

Comment 4

[Andrew McCreight [:mccr8]]

It would be nice if this could get fixed. It is the only remaining leak on non-Windows platforms, I believe.

In case it helps, the allocation stack for the nsThread is:

#00: nsThread::AddRef() (nsThread.cpp:181, in XUL)
#01: nsThreadManager::GetCurrentThread() (RefPtr.h:78, in XUL)
#02: nsThreadManager::GetCurrentThread(nsIThread**) (nsThreadManager.cpp:417, in XUL)
#03: mozilla::GetCurrentThreadEventTarget() (nsThreadUtils.cpp:176, in XUL)
#04: mozilla::ThreadEventTarget::Dispatch(already_AddRefed<nsIRunnable>, unsigned int) (ThreadEventTarget.cpp:142, in XUL)
#05: nsThread::Dispatch(already_AddRefed<nsIRunnable>, unsigned int) (AlreadyAddRefed.h:106, in XUL)
#06: nsIEventTarget::Dispatch(nsIRunnable*, unsigned int) (nsIEventTarget.h:37, in XUL)
#07: mozilla::WebrtcGmpVideoEncoder::Encode(webrtc::VideoFrame const&, webrtc::CodecSpecificInfo const*, std::__1::vector<webrtc::FrameType, std::__1::allocator<webrtc::FrameType> > const*) (WebrtcGmpVideoCodec.cpp:328, in XUL)
#08: webrtc::VCMGenericEncoder::Encode(webrtc::VideoFrame const&, webrtc::CodecSpecificInfo const*, std::__1::vector<webrtc::FrameType, std::__1::allocator<webrtc::FrameType> > const&) (generic_encoder.cc:70, in XUL)
#09: webrtc::vcm::VideoSender::AddVideoFrame(webrtc::VideoFrame const&, webrtc::CodecSpecificInfo const*) (video_sender.cc:350, in XUL)
#10: webrtc::ViEEncoder::EncodeVideoFrame(webrtc::VideoFrame const&, long long) (vie_encoder.cc:0, in XUL)
#11: webrtc::ViEEncoder::EncodeTask::Run() (vie_encoder.cc:115, in XUL)
#12: rtc::TaskQueue::TaskContext::RunTask(void*) (task_queue_gcd.cc:53, in XUL)
#13: _dispatch_client_callout (in libdispatch.dylib) + 8
...

Comment 5

[Randell Jesup [:jesup] (needinfo me)]

This is caused by using Dispatch() with NS_DISPATCH_SYNC in WebrtcGmpVideoEncoder::Encode(); that requires an nsThread, and this thread is a pthread created by webrtc.org

Switching it to match what's used for Decode() -- SyncRunnable, built explicitly for avoiding turning random threads into nsThreads - should fix it.

Comment 6

[Randell Jesup [:jesup] (needinfo me)]

Attachment: use SyncRunnable instead of Dispatch(...NS_DISPATCH_SYNC) when running on non-nsThreads

Comment 7

[Randell Jesup [:jesup] (needinfo me)]

 https://treeherder.mozilla.org/#/jobs?repo=try&revision=9dff425d7b504b3b4e8ef0d289a88c5318d42052

Comment 8

[Andrew McCreight [:mccr8]]

Thanks, Randell! That stack did look weird to me, but I wasn't sure if it was wrong or not.

Comment 9

[Andrew McCreight [:mccr8]]

Jan-Ivar, can you get to this review soon? I'd like to be able to reduce the leak threshold so further regressions like bug 1412408 don't land. Thanks.

Comment 10

[Andrew McCreight [:mccr8]]

Randell, is this ready to land?

Comment 11

[Randell Jesup [:jesup] (needinfo me)]

yes; doing a check-build before landing

Comment 12

[Andrew McCreight [:mccr8]]

I've done a bunch of try runs with this patch, so it should be okay to land.

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/e5b4f7ad5fc4
Use SyncRunnable instead of Dispatch(...NS_DISPATCH_SYNC) when running on non-nsThreads. r=jib

Comment 14

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/e5b4f7ad5fc4