Last Comment Bug 676376 - prevent multi-threaded JSRuntime access in new web worker memory reporters
: prevent multi-threaded JSRuntime access in new web worker memory reporters
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: unspecified
: All All
: -- normal (vote)
: mozilla8
Assigned To: Ben Turner (not reading bugmail, use the needinfo flag!)
:
Mentors:
Depends on:
Blocks: 650411
  Show dependency treegraph
 
Reported: 2011-08-03 14:12 PDT by Luke Wagner [:luke]
Modified: 2012-01-12 10:09 PST (History)
4 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Patch, v1 (15.99 KB, patch)
2011-08-06 12:32 PDT, Ben Turner (not reading bugmail, use the needinfo flag!)
luke: review+
jonas: review+
Details | Diff | Review

Description Luke Wagner [:luke] 2011-08-03 14:12:20 PDT
These were landed recently and block bug 650411 which wants to assert that all access to a JSRuntime comes from a single owning thread.

Fear not, bent has a plan!
Comment 1 Ben Turner (not reading bugmail, use the needinfo flag!) 2011-08-06 12:32:53 PDT
Created attachment 551272 [details] [diff] [review]
Patch, v1

Here's the best option I think. I went with this approach because we're about to split workers even further (for shared workers) and we can't touch those runtimes on the main thread any longer.
Comment 2 Jonas Sicking (:sicking) 2011-08-06 18:12:09 PDT
Comment on attachment 551272 [details] [diff] [review]
Patch, v1

Though would love a second r+ on this from luke.
Comment 3 Luke Wagner [:luke] 2011-08-08 09:19:38 PDT
Comment on attachment 551272 [details] [diff] [review]
Patch, v1

Looks good to me.  I confirmed that this does indeed fix the assert I was seeing previously.  Thanks!
Comment 4 Ben Turner (not reading bugmail, use the needinfo flag!) 2011-08-08 16:27:08 PDT
http://hg.mozilla.org/integration/mozilla-inbound/rev/65a834be2590
Comment 5 :Ehsan Akhgari (busy, don't ask for review please) 2011-08-09 09:01:15 PDT
http://hg.mozilla.org/mozilla-central/rev/65a834be2590
Comment 6 Bobby Holley (PTO through June 13) 2012-01-11 23:25:44 PST
So, I'm kind of confused here. Over in bug 716167, I assert that GetXPConnect is never called off-main-thread. This works everywhere except in toolkit/components/aboutmemory/tests/test_aboutmemory.xul, where I get crashes with the following stack:

CrashInJS () at js/src/jsutil.cpp:95
JS_Assert at /files/mozilla/repos/a/js/src/jsutil.cpp:115
nsXPConnect::GetXPConnect at js/xpconnect/src/nsXPConnect.cpp:173
nsXPConnect::GetRuntimeInstance at js/xpconnect/src/nsXPConnect.cpp:287
mozilla::xpconnect::memory::CollectCompartmentStatsForRuntime at js/xpconnect/src/XPCJSRuntime.cpp:1620
(anonymous namespace)::CollectRuntimeStatsRunnable::WorkerRun at dom/workers/WorkerPrivate.cpp:1480
mozilla::dom::workers::WorkerRunnable::Run at dom/workers/WorkerPrivate.cpp:1662
mozilla::dom::workers::WorkerPrivate::DoRunLoop at dom/workers/WorkerPrivate.cpp:2520
(anonymous namespace)::WorkerThreadRunnable::Run at dom/workers/RuntimeService.cpp:339
nsThread::ProcessNextEvent at xpcom/threads/nsThread.cpp:657
NS_ProcessNextEvent_P at /files/mozilla/build/a/xpcom/build/nsThreadUtils.cpp:245
nsThread::ThreadFunc at xpcom/threads/nsThread.cpp:289
_pt_root at nsprpub/pr/src/pthreads/ptthread.c:187
_pthread_start
thread_start

Is this expected? I haven't dug too much into the patch here, but it seems like the whole point of it is to make sure that stuff like this is happening on the main thread. So I'd be curious to know if this is a bug that I should be looking into, or whether I'm just misunderstanding something.
Comment 7 :Ms2ger 2012-01-12 01:18:45 PST
That's probably because of the double counting bug I fixed in bug 714264.
Comment 8 Bobby Holley (PTO through June 13) 2012-01-12 10:09:45 PST
(In reply to Ms2ger from comment #7)
> That's probably because of the double counting bug I fixed in bug 714264.

So it is! I just rebased and the crash went away.

You're the best, Ms2ger. I owe you some chocolate milk when we finally cross paths. ;-)

Note You need to log in before you can comment on or make changes to this bug.