LoadManager assumes that when a thread exits the runnable, the thread exits (thread leak)

RESOLVED FIXED in mozilla32

Status

()

defect
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: jesup, Assigned: jesup)

Tracking

(Depends on 1 bug)

Trunk
mozilla32
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

Needless to say, that's not how NewNamedThread/NewThread works, even if you provide an initial event.  We've dealt with in the past by having the runnable resend itself to MainThread to kill the thread.

Really we should look at the API for NewThread and in particular the "and provide an event" case.  Also, we should consider allowing threads to call Shutdown on themselves, even if behind the scenes (on some OSes?) it has to proxy it to MainThread.  But that's a separate question from this bug.
Depends on: 999104
Comment on attachment 8436545 [details] [diff] [review]
Properly shut down LoadMonitor threads

This is a straight pattern on what we did for AudioStream.cpp's Init thread and Crypto's nsCryptoTask.cpp.  Ugly but it's the working pattern for now.
Attachment #8436545 - Flags: review?(jib)
Comment on attachment 8436545 [details] [diff] [review]
Properly shut down LoadMonitor threads

Review of attachment 8436545 [details] [diff] [review]:
-----------------------------------------------------------------

lgtm.
Attachment #8436545 - Flags: review?(jib) → review+
https://hg.mozilla.org/mozilla-central/rev/b5067fbd502e
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
This push backed out for Android crashes during 408431-1.html:
https://tbpl.mozilla.org/?tree=Mozilla-Inbound&jobname=Android%204.0%20Panda%20mozilla-inbound%20opt%20test%20crashtest&fromchange=c66224db3600&tochange=6cc9ac1f57fa

I saw bug 1022235 comment 8 - however the existing 408431-1.html crashes were on other platforms - Android 4.0 debug m-6 was green before, with these two changesets, it isn't.

remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/dc93584a0948
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Relanded with agreement in IRC from edmorely - bug 1022235 is caused by bug 1022509
https://hg.mozilla.org/integration/mozilla-inbound/rev/97ffe21da653
Status: REOPENED → RESOLVED
Closed: 5 years ago5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.