1448230 - Intermittent leakcheck | default process: 1536 bytes leaked (CondVar, DataChannelConnection, DataChannelConnectionShutdown, Mutex, PollableEvent, ...)

Status: RESOLVED FIXED

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

Description

[Treeherder Bug Filer]

Filed by: ebalazs [at] mozilla.com

https://treeherder.mozilla.org/logviewer.html#?job_id=169804598&repo=mozilla-inbound

https://queue.taskcluster.net/v1/task/Vdqg1F_ESQirbmMxbdGCTg/runs/0/artifacts/public/logs/live_backing.log

Comment 1

[Intermittent Failures Robot]

40 failures in 822 pushes (0.049 failures/push) were associated with this bug in the last 7 days. 

This is the #40 most frequent failure this week.  

** This failure happened more than 30 times this week! Resolving this bug is a high priority. **

** Try to resolve this bug as soon as possible. If unresolved for 2 weeks, the affected test(s) may be disabled. **  

Repository breakdown:
* mozilla-inbound: 19
* autoland: 12
* mozilla-central: 6
* try: 3

Platform breakdown:
* linux32: 40

For more details, see:
https://treeherder.mozilla.org/intermittent-failures.html#/bugdetails?bug=1448230&startday=2018-03-20&endday=2018-03-26&tree=trunk

Comment 2

[Eliza Balazs [:ebalazs_]]

Since this bug was created, 5 days ago, we have 40 failures.
They occur on Linux32 debug.

Recent failure log example: https://treeherder.mozilla.org/logviewer.html#?repo=mozilla-inbound&job_id=170508324&lineNumber=4471

and a relevant part of it:

[task 2018-03-27T10:52:06.157Z] 10:52:06     INFO - TEST-INFO | leakcheck | default process: leaked 1 nsTimerImpl
[task 2018-03-27T10:52:06.158Z] 10:52:06    ERROR - TEST-UNEXPECTED-FAIL | leakcheck | default process: 1536 bytes leaked (CondVar, DataChannelConnection, DataChannelConnectionShutdown, Mutex, PollableEvent, ...)
[task 2018-03-27T10:52:06.158Z] 10:52:06     INFO - runtests.py | Running tests: end.
[task 2018-03-27T10:52:06.174Z] 10:52:06     INFO - Buffered messages logged at 10:51:57

:overholt, can you please have a look at this?

Comment 3

[Andrew Overholt [:overholt]]

Next week I'll find someone to investigate. Keeping needinfo.

Comment 4

[Intermittent Failures Robot]

28 failures in 718 pushes (0.039 failures/push) were associated with this bug in the last 7 days.    

Repository breakdown:
* mozilla-inbound: 12
* autoland: 9
* try: 4
* mozilla-central: 3

Platform breakdown:
* linux32: 28

For more details, see:
https://treeherder.mozilla.org/intermittent-failures.html#/bugdetails?bug=1448230&startday=2018-03-26&endday=2018-04-01&tree=trunk

Comment 5

[Andrew Overholt [:overholt]]

(In reply to Andrew Overholt [:overholt] from comment #3)
> Next week I'll find someone to investigate. Keeping needinfo.

"Next week" ...

I'm not sure what's up here. It looks like something is going wrong with a test expecting a worker to still be alive but that might be a red herring. Mats, authored most of https://searchfox.org/mozilla-central/source/dom/events/test/test_bug322588.html which *may* be related here. Maybe he can take a quick look?

Comment 6

[Mats Palmgren (inactive)]

I don't see any workers in that test, it appears to open/close
a window from a click event.  I don't know why that would cause
a leak related to DataChannelConnection.  I doubt this bug is
in any code I have knowledge about, sorry.

Comment 7

[Olli Pettay [:smaug][[e-mail]]]

Only DataChannelConnection related stuff is leaking. Smells like WebRTC

Comment 8

[Dan Minor [:dminor]]

Randell is the expert on data channels, but he's not actively working on WebRTC these days. I'm going to move the needinfo to Nils to see if we can find someone else to investigate this first.

Comment 9

[Nils Ohlmeier [:drno]]

I actually created that problem here https://searchfox.org/mozilla-central/source/dom/events/test/test_all_synthetic_events.html#292
when I landed bug 1447692

Comment 10

[Nils Ohlmeier [:drno]]

Attachment: Bug 1448230: close PeerConnection to prevent leaking.

Review commit: https://reviewboard.mozilla.org/r/238266/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/238266/

Comment 11

[Nils Ohlmeier [:drno]]

I'm actually concerned that closing the PeerConnection is duct taping over something...

Which makes me wonder why this is failing on 32 bit Linux only.

Comment 12

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

Well, timing is considerably different there for a number of reasons, so likely there's a race with the shutdown somewhere.  

And the thing that is leaking is the DataChannelConnection itself.  Perhaps this is due to the 30 second timing in the shutdown callback (and we leaked one of those, and a timer).  It would be good to get rid of that.

Comment 13

[Nils Ohlmeier [:drno]]

Comment on attachment 8969506 [details]
Bug 1448230: close PeerConnection to prevent leaking.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/238266/diff/1-2/

Comment 14

[Jan-Ivar Bruaroey [:jib] (needinfo? me)]

(In reply to Randell Jesup [:jesup] from comment #12)
> And the thing that is leaking is the DataChannelConnection itself.  Perhaps
> this is due to the 30 second timing in the shutdown callback (and we leaked
> one of those, and a timer).  It would be good to get rid of that.

That seems likely. we never cancel the timer, except in the destructor [1] of the nsITimerCallback, which will never happen since presumably the timer holds a refcount to it? We should probably explicitly cancel the timer when we no longer need it.

[1] https://searchfox.org/mozilla-central/rev/8f06c1b9a080b84435a2906e420fe102e1ed780b/netwerk/sctp/datachannel/DataChannel.cpp#92

Comment 15

[Jan-Ivar Bruaroey [:jib] (needinfo? me)]

Comment on attachment 8969506 [details]
Bug 1448230: close PeerConnection to prevent leaking.

https://reviewboard.mozilla.org/r/238266/#review244986

This is a cleaner use of peer connection for the purpose of just testing an event. But it will bury the leak, so I suggest we fix the leak before landing this.

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/2a51222fb823
close PeerConnection to prevent leaking. r=jib

Comment 17

[Raul Gurzau (:RaulG)]

https://hg.mozilla.org/mozilla-central/rev/2a51222fb823