Closed Bug 676376 Opened 11 years ago Closed 11 years ago

prevent multi-threaded JSRuntime access in new web worker memory reporters


(Core :: JavaScript Engine, defect)

Not set





(Reporter: luke, Assigned: bent.mozilla)




(1 file)

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!
Attached patch Patch, v1Splinter Review
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.
Attachment #551272 - Flags: review?(luke)
Comment on attachment 551272 [details] [diff] [review]
Patch, v1

Though would love a second r+ on this from luke.
Attachment #551272 - Flags: review?(jonas) → review+
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!
Attachment #551272 - Flags: review?(luke) → review+
Closed: 11 years ago
Resolution: --- → FIXED
Whiteboard: [inbound]
Target Milestone: --- → mozilla8
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

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.
That's probably because of the double counting bug I fixed in bug 714264.
(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. ;-)
You need to log in before you can comment on or make changes to this bug.