Closed
Bug 1406328
Opened 3 years ago
Closed 3 years ago
Don't shut down MediaCache::mThread too early
Categories
(Core :: Audio/Video: Playback, enhancement, P3)
Core
Audio/Video: Playback
Tracking
()
RESOLVED
FIXED
mozilla58
Tracking | Status | |
---|---|---|
firefox58 | --- | fixed |
People
(Reporter: jwwang, Assigned: jwwang)
References
Details
Attachments
(1 file)
https://reviewboard-hg.mozilla.org/gecko/rev/228b08c8c93437a938ca3e3b6f24bf597690e65d#l2.52 It is possible for an nsIChannel to live longer than a MediaCache. Shutting down the thread too early will cause Dispatch() to fail [1] and result in leaks [2]. [1] http://searchfox.org/mozilla-central/rev/8efd128b48cdae3a6c6862795ce618aa1ca1c0b4/netwerk/ipc/ChannelEventQueue.cpp#178 [2] http://searchfox.org/mozilla-central/rev/8efd128b48cdae3a6c6862795ce618aa1ca1c0b4/xpcom/threads/ThreadEventTarget.cpp#128
Assignee | ||
Updated•3 years ago
|
Comment hidden (mozreview-request) |
Assignee | ||
Updated•3 years ago
|
Attachment #8916467 -
Flags: review?(gsquelart)
Comment 2•3 years ago
|
||
mozreview-review |
Comment on attachment 8916467 [details] Bug 1406328 - shut down the MediaCache thread in ShutdownThreads phase. https://reviewboard.mozilla.org/r/187586/#review192594 Clever way to (ab)use ClearOnShutdown! :-) ::: commit-message-50846:1 (Diff revision 1) > +Bug 1406328 - shut down the MediaCache thread in ShutdownThreads phase. > + > +To avoid leaks caused by Dispatch() failures. See comment 0 for the detail. I think you should also mention that this patch introduces a new (and I think, better) behavior: Only one thread is ever created, and is shared between all MediaCaches.
Attachment #8916467 -
Flags: review?(gsquelart) → review+
Comment hidden (mozreview-request) |
Assignee | ||
Comment 4•3 years ago
|
||
Thanks!
Pushed by jwwang@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/3ceb919e7e82 shut down the MediaCache thread in ShutdownThreads phase. r=gerald
Backed out for build bustage at dom/media/MediaCache.cpp:301: 'ShutdownPhase' has not been declared: https://hg.mozilla.org/integration/autoland/rev/224b59f8a361ddbd51489dc3b74b9d53e66f1df2 Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=3ceb919e7e829c358e18ca234c1a100871e29963&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=retry&filter-resultStatus=usercancel&filter-resultStatus=runnable Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=135626892&repo=autoland /builds/worker/workspace/build/src/dom/media/MediaCache.cpp:301:29: error: 'ShutdownPhase' has not been declared /builds/worker/workspace/build/src/dom/media/MediaCache.cpp:301:59: error: 'ClearOnShutdown' was not declared in this scope
Flags: needinfo?(jwwang)
Comment hidden (mozreview-request) |
Assignee | ||
Updated•3 years ago
|
Flags: needinfo?(jwwang)
Pushed by jwwang@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/2e858715590b shut down the MediaCache thread in ShutdownThreads phase. r=gerald
![]() |
||
Comment 9•3 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/2e858715590b
Status: NEW → RESOLVED
Closed: 3 years ago
status-firefox58:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla58
You need to log in
before you can comment on or make changes to this bug.
Description
•