Closed Bug 1045847 Opened 5 years ago Closed 5 years ago

nsTraceRefcnt gets initialized before sChildProcessType

Categories

(Core :: XPCOM, defect)

x86_64
Linux
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla34

People

(Reporter: billm, Assigned: billm)

References

Details

(Whiteboard: [MemShrink:P2])

Attachments

(1 file)

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.
Whiteboard: [MemShrink]
Attached patch process-typeSplinter Review
This seems like a more direct route to me.
Assignee: nobody → wmccloskey
Status: NEW → ASSIGNED
Attachment #8464294 - Flags: review?(khuey)
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.
Attachment #8464294 - Flags: review?(khuey) → review+
Whiteboard: [MemShrink] → [MemShrink:P2]
billm: ready to check in?
Flags: needinfo?(wmccloskey)
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.
Flags: needinfo?(wmccloskey)
https://hg.mozilla.org/mozilla-central/rev/cde0f665b16a
https://hg.mozilla.org/mozilla-central/rev/e65a6c49e46e
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla34
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.
Flags: needinfo?(wmccloskey)
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.
Flags: needinfo?(wmccloskey)
You need to log in before you can comment on or make changes to this bug.