Closed Bug 1163103 Opened 10 years ago Closed 6 years ago

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

Categories

(Core :: XPCOM, defect)

Product:
Core
Component:
XPCOM
Type:
defect
Priority:
Not set
Severity:
major

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla70
Tracking Flags:
Tracking Status
firefox70 --- fixed

People

(Reporter: jib, Assigned: Gijs)

References

Details

Attachments

(1 file)

Bug 1163103 - stop using NS_GetSpecialDirectory off the main thread within the cycle collector (via nsDumpUtils.cpp), r?mccr8
47 bytes, text/x-phabricator-request
Details | Review
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 ()
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 ()
Component: about:memory → XPCOM
Product: Toolkit → Core
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?
(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.
Assignee: nobody → gijskruitbosch+bugs
Status: NEW → ASSIGNED
Pushed by gijskruitbosch@gmail.com: 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

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

Status: ASSIGNED → RESOLVED
Closed: 6 years ago
status-firefox70: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla70
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: