1045847 - nsTraceRefcnt gets initialized before sChildProcessType

Status: RESOLVED FIXED

(Core :: XPCOM, defect)

Description

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

As a consequence, calls to XRE_GetProcessType() in nsTraceRefcnt return the wrong value. This causes us to use the same refcount log file for all our processes so we're missing leaks. Here's the stack for when nsTraceRefcount gets initialized:

#2  0x00007ffff122d570 in InitTraceLog () at /home/billm/mozilla/in1/xpcom/base/nsTraceRefcnt.cpp:738
#3  0x00007ffff122e266 in NS_LogCtor (aPtr=0x7fffe864b078, aType=0x7ffff58aa27b "nsTArray_base", aInstanceSize=8)
    at /home/billm/mozilla/in1/xpcom/base/nsTraceRefcnt.cpp:1124
#4  0x00007ffff11be254 in nsTArray_base<nsTArrayInfallibleAllocator, nsTArray_CopyWithMemutils>::nsTArray_base (
    this=0x7fffe864b078) at ../../dist/include/nsTArray-inl.h:15
#5  0x00007ffff12d5c3e in nsTArray_Impl<PLHashEntry*, nsTArrayInfallibleAllocator>::nsTArray_Impl (this=
    0x7fffe864b078) at /home/billm/mozilla/in1/xpcom/build/../glue/nsTArray.h:736
#6  0x00007ffff12d58c2 in nsTArray<PLHashEntry*>::nsTArray (this=0x7fffe864b078)
    at /home/billm/mozilla/in1/xpcom/build/../glue/nsTArray.h:1707
#7  0x00007ffff12d53f3 in mozilla::DeadlockDetector<mozilla::BlockingResourceBase::DeadlockDetectorEntry>::OrderingEntry::OrderingEntry (this=0x7fffe864b070) at ../../dist/include/mozilla/DeadlockDetector.h:196
#8  0x00007ffff12d4a4a in mozilla::DeadlockDetector<mozilla::BlockingResourceBase::DeadlockDetectorEntry>::PutEntry
    (this=0x7fffe864b060, aKey=0x7fffe860b740) at ../../dist/include/mozilla/DeadlockDetector.h:243
#9  0x00007ffff12d3e10 in mozilla::DeadlockDetector<mozilla::BlockingResourceBase::DeadlockDetectorEntry>::Add (
    this=0x7fffe864b060, aResource=0x7fffe860b740) at ../../dist/include/mozilla/DeadlockDetector.h:368
#10 0x00007ffff12cc5c7 in mozilla::BlockingResourceBase::BlockingResourceBase (this=0x7fffe860b640, aName=
    0x7ffff5ff00d0 "sRegisteredThreads mutex", aType=mozilla::BlockingResourceBase::eMutex)
    at /home/billm/mozilla/in1/xpcom/glue/BlockingResourceBase.cpp:82
#11 0x00007ffff11cb260 in mozilla::OffTheBooksMutex::OffTheBooksMutex (this=0x7fffe860b640, aName=
    0x7ffff5ff00d0 "sRegisteredThreads mutex") at ../../dist/include/mozilla/Mutex.h:47
#12 0x00007ffff11cb32b in mozilla::Mutex::Mutex (this=0x7fffe860b640, aName=
    0x7ffff5ff00d0 "sRegisteredThreads mutex") at ../../dist/include/mozilla/Mutex.h:124
#13 0x00007ffff41f78b5 in Sampler::Startup () at /home/billm/mozilla/in1/tools/profiler/platform.cpp:84
#14 0x00007ffff41f8ef3 in mozilla_sampler_init (stackTop=0x7fffffffd98d)
    at /home/billm/mozilla/in1/tools/profiler/platform.cpp:508
#15 0x00007ffff42ea006 in profiler_init (stackTop=0x7fffffffd98d) at ../../dist/include/GeckoProfilerImpl.h:64
#16 0x00007ffff42eab7e in XRE_InitChildProcess (aArgc=5, aArgv=0x7fffffffdcf8, aProcess=GeckoProcessType_Content)
    at /home/billm/mozilla/in1/toolkit/xre/nsEmbedFunctions.cpp:306
#17 0x0000000000403ba2 in main (argc=5, argv=0x7fffffffdcf8)
    at /home/billm/mozilla/in1/ipc/app/MozillaRuntimeMain.cpp:147

We might be able to move the sChildProcessType initialization up before the profiler is set up, but I'm a little worried about some of the stuff in main() that happens even before that.

Comment 1

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

Attachment: process-type

This seems like a more direct route to me.

Comment 2

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Comment on attachment 8464294 [details] [diff] [review]
process-type

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

::: ipc/app/MozillaRuntimeMain.cpp
@@ +85,5 @@
>  {
> +    // Check for the absolute minimum number of args we need to move
> +    // forward here. We expect the last arg to be the child process type.
> +    if (argc < 1)
> +      return 3;

braces please

::: toolkit/xre/nsEmbedFunctions.cpp
@@ +205,5 @@
> +}
> +}
> +
> +void
> +XRE_SetProcessType(const char* aProcessTypeString)

Can you make this MOZ_CRASH if it's called again?  Seems like it would be useful in exploits to call this and try to upgrade yourself to be the parent process.

Comment 3

[Nicholas Nethercote [inactive]]

billm: ready to check in?

Comment 4

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

It looks like the leak that was blocking this went away. I'll push it to try and see if it's ready to go.

Comment 5

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/cde0f665b16a
https://hg.mozilla.org/integration/mozilla-inbound/rev/e65a6c49e46e

Comment 6

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/cde0f665b16a
https://hg.mozilla.org/mozilla-central/rev/e65a6c49e46e

Comment 7

[Andrew McCreight [:mccr8]]

Bill, could you file a bug about making sure that shutdown leak logging works with e10s and CC me?  That's kind of a blocker to switching over to e10s for reals.

Comment 8

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

I filed bug 1051230. All the logging code works fine. It's just that content processes crash before the leak log is printed and we don't detect that.