Closed Bug 676376 Opened 8 years ago Closed 8 years ago
prevent multi-threaded JSRuntime access in new web worker memory reporters
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!
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)
8 years ago
Attachment #551272 - Flags: review?(jonas)
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+
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
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 _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.
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.