Closed Bug 862500 Opened 7 years ago Closed 7 years ago

Use RAII to shutdown profiler

Categories

(Core :: Gecko Profiler, defect)

x86
macOS
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla23

People

(Reporter: BenWa, Assigned: BenWa)

References

Details

Attachments

(2 files, 3 obsolete files)

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
Attached 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
Attached 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.
Attached patch patch (obsolete) — Splinter Review
https://tbpl.mozilla.org/?tree=Try&rev=796a5192dc4b
Attachment #738134 - Attachment is obsolete: true
Attachment #738501 - Flags: review+
Attached 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.
Attached 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: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla23
You need to log in before you can comment on or make changes to this bug.