862500 - Use RAII to shutdown profiler

Status: RESOLVED FIXED

(Core :: Gecko Profiler, defect)

Description

[Benoit Girard (:BenWa)]

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.

Comment 1

[Benoit Girard (:BenWa)]

Attachment: patch

https://tbpl.mozilla.org/?tree=Try&rev=8551a30c53b3

Comment 2

[James Willcox (:snorp) ([e-mail]) (he/him)]

Comment on attachment 738134 [details] [diff] [review]
patch

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

Great

Comment 3

[Benoit Girard (:BenWa)]

https://tbpl.mozilla.org/?tree=Try&rev=99335c3cfe23
oops

Comment 4

[Benoit Girard (:BenWa)]

infrastructure failure, trying again:
https://tbpl.mozilla.org/?tree=Try&rev=c2f3d23baa49

Comment 5

[Benoit Girard (:BenWa)]

Blah, it was my fault. Here's a good fix:
https://tbpl.mozilla.org/?tree=Try&rev=46523c3e1bc1

Comment 6

[Benoit Girard (:BenWa)]

Attachment: 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 [e-mail] 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.

Comment 7

[Benoit Girard (:BenWa)]

Attachment: patch

https://tbpl.mozilla.org/?tree=Try&rev=796a5192dc4b

Comment 8

[Benoit Girard (:BenWa)]

Attachment: patch

This patch didn't handle xpcom init properly. Trying again:
https://tbpl.mozilla.org/?tree=Try&rev=cdea62c550c9

Comment 9

[Benoit Girard (:BenWa)]

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

Comment 10

[Benoit Girard (:BenWa)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/4444510d672f

Comment 11

[Ryan VanderMeulen [:RyanVM]]

Backed out along with bug 734691 for leaks.
https://hg.mozilla.org/integration/mozilla-inbound/rev/fa5d5fccbc11

Comment 12

[Benoit Girard (:BenWa)]

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.

Comment 13

[Benoit Girard (:BenWa)]

Attachment: patch

Minor tweak to make sure profiler_shutdown is called after NS_LogTerm.

Comment 14

[Benoit Girard (:BenWa)]

before*

Comment 15

[Benoit Girard (:BenWa)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/c0e86fd53a37

Comment 16

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/c0e86fd53a37