Closed Bug 1448230 Opened 6 years ago Closed 6 years ago

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

Categories

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

Product:
Core
Component:
WebRTC: Networking
Type:
defect

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla61
Tracking Flags:
Tracking Status
firefox61 --- fixed

People

(Reporter: intermittent-bug-filer, Assigned: drno)

References

Details

(Keywords: intermittent-failure, Whiteboard: [stockwell fixed:other])

Attachments

(1 file)

Bug 1448230: close PeerConnection to prevent leaking.
59 bytes, text/x-review-board-request
jib
: review+
Details
Component: General → DOM: Events
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?
Flags: needinfo?(overholt)
Next week I'll find someone to investigate. Keeping needinfo.
(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?
Flags: needinfo?(overholt) → needinfo?(mats)
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.
Flags: needinfo?(mats)
Component: DOM: Events → DOM
Only DataChannelConnection related stuff is leaking. Smells like WebRTC
Component: DOM → WebRTC: Networking
Flags: needinfo?(rjesup)
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.
Rank: 15
Flags: needinfo?(rjesup) → needinfo?(drno)
Priority: -- → P2
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
Assignee: nobody → drno
Depends on: 1447692
Flags: needinfo?(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.
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.
(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 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.
Attachment #8969506 - Flags: review?(jib) → review+
Pushed by drno@ohlmeier.org:
https://hg.mozilla.org/integration/autoland/rev/2a51222fb823
close PeerConnection to prevent leaking. r=jib
https://hg.mozilla.org/mozilla-central/rev/2a51222fb823
Status: NEW → RESOLVED
Closed: 6 years ago
status-firefox61: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla61
Whiteboard: [stockwell needswork:owner] → [stockwell fixed:other]
You need to log in before you can comment on or make changes to this bug.