Use RAII to shutdown profiler

RESOLVED FIXED in mozilla23

Status

()

defect
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: BenWa, Assigned: BenWa)

Tracking

unspecified
mozilla23
x86
macOS
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 3 obsolete attachments)

bug 734691 just got backed out because if we crash we leak the mutex. I think that's is happening is we must hit some early return in XRE_main and friends that skips profiler_shutdown.

I plan on just using RAII to enforce a shutdown and reland bug 734691.
Blocks: 734691
Posted patch patch (obsolete) — Splinter Review
https://tbpl.mozilla.org/?tree=Try&rev=8551a30c53b3
Assignee: nobody → bgirard
Status: NEW → ASSIGNED
Attachment #738134 - Flags: review?(snorp)
Comment on attachment 738134 [details] [diff] [review]
patch

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

Great
Attachment #738134 - Flags: review?(snorp) → review+
infrastructure failure, trying again:
https://tbpl.mozilla.org/?tree=Try&rev=c2f3d23baa49
Blah, it was my fault. Here's a good fix:
https://tbpl.mozilla.org/?tree=Try&rev=46523c3e1bc1
Posted file Trace
Ok found the problem, we previously leak the profiler for XPCShell test. This patch fixes that. The problem is we destroy a thread after the main method as exited after which we call mozilla::ShutdownXPCOM. We can't hold the profiler open in main@xpcshell.cpp and I don't really feel like exporting the symbol out of libxul. We thus just need to recover from unregister thread being called after shutdown.
Posted patch patch (obsolete) — Splinter Review
https://tbpl.mozilla.org/?tree=Try&rev=796a5192dc4b
Attachment #738134 - Attachment is obsolete: true
Attachment #738501 - Flags: review+
Posted patch patch (obsolete) — Splinter Review
This patch didn't handle xpcom init properly. Trying again:
https://tbpl.mozilla.org/?tree=Try&rev=cdea62c550c9
Attachment #738501 - Attachment is obsolete: true
Attachment #738589 - Flags: review+
I had to rebase 300 lines by hand of non trivial bit rot in platform-linx.cc so I'm checking I didn't break the build:
https://tbpl.mozilla.org/?tree=Try&rev=d09146cde2da
Ahh this is really dumb. Our startup/shutdown case is violently special for every run time configuration.

We use ScopedLogging everywhere expect for plugins. In this case the profiler was getting destroyed RIGHT after we printed the leakstats. I'll be relanding with ScopedLogging for plugin shortly.
Posted patch patchSplinter Review
Minor tweak to make sure profiler_shutdown is called after NS_LogTerm.
Attachment #738589 - Attachment is obsolete: true
Attachment #739092 - Flags: review+
before*
https://hg.mozilla.org/mozilla-central/rev/c0e86fd53a37
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla23
You need to log in before you can comment on or make changes to this bug.