1163103 - nsCycleCollectorLogger calls nsDirectoryService::Get off main-thread on shutdown (unsafe race)

Status: RESOLVED FIXED

(Core :: XPCOM, defect)

Description

[Jan-Ivar Bruaroey [:jib] (needinfo? me)]

STR: Start and quit firefox with patch in Bug 1163021 comment 8.

In thread DOM Worker (34) here http://mxr.mozilla.org/mozilla-central/source/xpcom/base/nsCycleCollector.cpp?mark=1629-1630#1607

This is unsafe as pointed out in Bug 1163021 comment 2, and could be causing crashes on shutdown.

DOM Worker (34)
#0	0x00000001016c2014 in nsDirectoryService::Get(char const*, nsID const&, void**) at /Users/Jan/moz/mozilla-central/xpcom/io/nsDirectoryService.cpp:364
#1	0x00000001016c29da in non-virtual thunk to nsDirectoryService::Get(char const*, nsID const&, void**) at /Users/Jan/moz/mozilla-central/xpcom/io/nsDirectoryService.cpp:407
#2	0x0000000101640d19 in NS_GetSpecialDirectory(char const*, nsIFile**) at /Users/Jan/moz/mozilla-central/obj-x86_64-apple-darwin12.2.1-debug/xpcom/base/../../dist/include/nsDirectoryServiceUtils.h:27
#3	0x0000000101637605 in nsDumpUtils::OpenTempFile(nsACString_internal const&, nsIFile**, nsACString_internal const&) at /Users/Jan/moz/mozilla-central/xpcom/base/nsDumpUtils.cpp:452
#4	0x000000010164ed59 in nsCycleCollectorLogSinkToFile::CreateTempFile(char const*) at /Users/Jan/moz/mozilla-central/xpcom/base/nsCycleCollector.cpp:1629
#5	0x000000010164f762 in nsCycleCollectorLogSinkToFile::OpenLog(nsCycleCollectorLogSinkToFile::FileInfo*) at /Users/Jan/moz/mozilla-central/xpcom/base/nsCycleCollector.cpp:1650
#6	0x000000010164190e in nsCycleCollectorLogSinkToFile::Open(__sFILE**, __sFILE**) at /Users/Jan/moz/mozilla-central/xpcom/base/nsCycleCollector.cpp:1549
#7	0x0000000101641f57 in nsCycleCollectorLogger::Begin() at /Users/Jan/moz/mozilla-central/xpcom/base/nsCycleCollector.cpp:1802
#8	0x000000010163426e in nsCycleCollector::BeginCollection(ccType, nsICycleCollectorListener*) at /Users/Jan/moz/mozilla-central/xpcom/base/nsCycleCollector.cpp:3775
#9	0x0000000101633c48 in nsCycleCollector::Collect(ccType, js::SliceBudget&, nsICycleCollectorListener*, bool) at /Users/Jan/moz/mozilla-central/xpcom/base/nsCycleCollector.cpp:3604
#10	0x0000000101633ab6 in nsCycleCollector::ShutdownCollect() at /Users/Jan/moz/mozilla-central/xpcom/base/nsCycleCollector.cpp:3556
#11	0x0000000101634a83 in nsCycleCollector::Shutdown() at /Users/Jan/moz/mozilla-central/xpcom/base/nsCycleCollector.cpp:3823
#12	0x0000000101635907 in nsCycleCollector_shutdown() at /Users/Jan/moz/mozilla-central/xpcom/base/nsCycleCollector.cpp:4146
#13	0x0000000104c6032a in (anonymous namespace)::WorkerJSRuntime::~WorkerJSRuntime() at /Users/Jan/moz/mozilla-central/dom/workers/RuntimeService.cpp:980
#14	0x0000000104c5fce5 in (anonymous namespace)::WorkerJSRuntime::~WorkerJSRuntime() at /Users/Jan/moz/mozilla-central/dom/workers/RuntimeService.cpp:969
#15	0x0000000104c5f2df in (anonymous namespace)::WorkerThreadPrimaryRunnable::Run() at /Users/Jan/moz/mozilla-central/dom/workers/RuntimeService.cpp:2833
#16	0x000000010171da28 in nsThread::ProcessNextEvent(bool, bool*) at /Users/Jan/moz/mozilla-central/xpcom/threads/nsThread.cpp:868
#17	0x000000010177a7c7 in NS_ProcessNextEvent(nsIThread*, bool) at /Users/Jan/moz/mozilla-central/xpcom/glue/nsThreadUtils.cpp:265
#18	0x0000000101dca3a3 in mozilla::ipc::MessagePumpForNonMainThreads::Run(base::MessagePump::Delegate*) at /Users/Jan/moz/mozilla-central/ipc/glue/MessagePump.cpp:326
#19	0x0000000101d3cb35 in MessageLoop::RunInternal() at /Users/Jan/moz/mozilla-central/ipc/chromium/src/base/message_loop.cc:233
#20	0x0000000101d3ca45 in MessageLoop::RunHandler() at /Users/Jan/moz/mozilla-central/ipc/chromium/src/base/message_loop.cc:226
#21	0x0000000101d3c9ed in MessageLoop::Run() at /Users/Jan/moz/mozilla-central/ipc/chromium/src/base/message_loop.cc:200
#22	0x000000010171bee6 in nsThread::ThreadFunc(void*) at /Users/Jan/moz/mozilla-central/xpcom/threads/nsThread.cpp:364
#23	0x0000000101376d2f in _pt_root at /Users/Jan/moz/mozilla-central/nsprpub/pr/src/pthreads/ptthread.c:212
#24	0x00007fff89375899 in _pthread_body ()
#25	0x00007fff8937572a in _pthread_start ()
#26	0x00007fff89379fc9 in thread_start ()

Comment 1

[Jan-Ivar Bruaroey [:jib] (needinfo? me)]

Meant to format that better:
> DOM Worker (34)
> #0	0x00000001016c2014 in nsDirectoryService::Get(char const*, nsID const&, void**) at /Users/Jan/moz/mozilla-central/xpcom/io/nsDirectoryService.cpp:364
> #1	0x00000001016c29da in non-virtual thunk to nsDirectoryService::Get(char const*, nsID const&, void**) at /Users/Jan/moz/mozilla-central/xpcom/io/nsDirectoryService.cpp:407
> #2	0x0000000101640d19 in NS_GetSpecialDirectory(char const*, nsIFile**) at /Users/Jan/moz/mozilla-central/obj-x86_64-apple-darwin12.2.1-debug/xpcom/base/../../dist/include/nsDirectoryServiceUtils.h:27
> #3	0x0000000101637605 in nsDumpUtils::OpenTempFile(nsACString_internal const&, nsIFile**, nsACString_internal const&) at /Users/Jan/moz/mozilla-central/xpcom/base/nsDumpUtils.cpp:452
> #4	0x000000010164ed59 in nsCycleCollectorLogSinkToFile::CreateTempFile(char const*) at /Users/Jan/moz/mozilla-central/xpcom/base/nsCycleCollector.cpp:1629
> #5	0x000000010164f762 in nsCycleCollectorLogSinkToFile::OpenLog(nsCycleCollectorLogSinkToFile::FileInfo*) at /Users/Jan/moz/mozilla-central/xpcom/base/nsCycleCollector.cpp:1650
> #6	0x000000010164190e in nsCycleCollectorLogSinkToFile::Open(__sFILE**, __sFILE**) at /Users/Jan/moz/mozilla-central/xpcom/base/nsCycleCollector.cpp:1549
> #7	0x0000000101641f57 in nsCycleCollectorLogger::Begin() at /Users/Jan/moz/mozilla-central/xpcom/base/nsCycleCollector.cpp:1802
> #8	0x000000010163426e in nsCycleCollector::BeginCollection(ccType, nsICycleCollectorListener*) at /Users/Jan/moz/mozilla-central/xpcom/base/nsCycleCollector.cpp:3775
> #9	0x0000000101633c48 in nsCycleCollector::Collect(ccType, js::SliceBudget&, nsICycleCollectorListener*, bool) at /Users/Jan/moz/mozilla-central/xpcom/base/nsCycleCollector.cpp:3604
> #10	0x0000000101633ab6 in nsCycleCollector::ShutdownCollect() at /Users/Jan/moz/mozilla-central/xpcom/base/nsCycleCollector.cpp:3556
> #11	0x0000000101634a83 in nsCycleCollector::Shutdown() at /Users/Jan/moz/mozilla-central/xpcom/base/nsCycleCollector.cpp:3823
> #12	0x0000000101635907 in nsCycleCollector_shutdown() at /Users/Jan/moz/mozilla-central/xpcom/base/nsCycleCollector.cpp:4146
> #13	0x0000000104c6032a in (anonymous namespace)::WorkerJSRuntime::~WorkerJSRuntime() at /Users/Jan/moz/mozilla-central/dom/workers/RuntimeService.cpp:980
> #14	0x0000000104c5fce5 in (anonymous namespace)::WorkerJSRuntime::~WorkerJSRuntime() at /Users/Jan/moz/mozilla-central/dom/workers/RuntimeService.cpp:969
> #15	0x0000000104c5f2df in (anonymous namespace)::WorkerThreadPrimaryRunnable::Run() at /Users/Jan/moz/mozilla-central/dom/workers/RuntimeService.cpp:2833
> #16	0x000000010171da28 in nsThread::ProcessNextEvent(bool, bool*) at /Users/Jan/moz/mozilla-central/xpcom/threads/nsThread.cpp:868
> #17	0x000000010177a7c7 in NS_ProcessNextEvent(nsIThread*, bool) at /Users/Jan/moz/mozilla-central/xpcom/glue/nsThreadUtils.cpp:265
> #18	0x0000000101dca3a3 in mozilla::ipc::MessagePumpForNonMainThreads::Run(base::MessagePump::Delegate*) at /Users/Jan/moz/mozilla-central/ipc/glue/MessagePump.cpp:326
> #19	0x0000000101d3cb35 in MessageLoop::RunInternal() at /Users/Jan/moz/mozilla-central/ipc/chromium/src/base/message_loop.cc:233
> #20	0x0000000101d3ca45 in MessageLoop::RunHandler() at /Users/Jan/moz/mozilla-central/ipc/chromium/src/base/message_loop.cc:226
> #21	0x0000000101d3c9ed in MessageLoop::Run() at /Users/Jan/moz/mozilla-central/ipc/chromium/src/base/message_loop.cc:200
> #22	0x000000010171bee6 in nsThread::ThreadFunc(void*) at /Users/Jan/moz/mozilla-central/xpcom/threads/nsThread.cpp:364
> #23	0x0000000101376d2f in _pt_root at /Users/Jan/moz/mozilla-central/nsprpub/pr/src/pthreads/ptthread.c:212
> #24	0x00007fff89375899 in _pthread_body ()
> #25	0x00007fff8937572a in _pthread_start ()
> #26	0x00007fff89379fc9 in thread_start ()

Comment 2

[Jed Davis [:jld] ⟨⏰|UTC-7⟩ ⟦he/him⟧]

I guess nsDumpUtils::OpenTempFile needs to send a sync runnable to the main thread?

Also, does e10s-enabled cycle collection interact with worker CC in such a way that this could be a problem there?

Comment 3

[Andrew McCreight [:mccr8]]

(In reply to Jed Davis [:jld] {UTC-7} from comment #2)
> Also, does e10s-enabled cycle collection interact with worker CC in such a
> way that this could be a problem there?

Main thread CC should never interact with worker CC, so far as I remember.

Comment 4

[:Gijs (he/him)]

Attachment: Bug 1163103 - stop using NS_GetSpecialDirectory off the main thread within the cycle collector (via nsDumpUtils.cpp), r?mccr8

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/52cd8205384f
stop using NS_GetSpecialDirectory off the main thread within the cycle collector (via nsDumpUtils.cpp), r=mccr8

Comment 6

[Andrei Ciure[:aciure]]

https://hg.mozilla.org/mozilla-central/rev/52cd8205384f