Closed Bug 507783 Opened 15 years ago Closed 14 years ago

Firebug needs some way to associate a Worker with the window that created it.

Categories

(Core :: General, defect)

x86
Windows XP
defect
Not set
normal

Tracking

()

RESOLVED INVALID

People

(Reporter: johnjbarton, Unassigned)

References

Details

(Whiteboard: [firebug-p2])

When JS code creates a Worker, Firebug can see the compilation of the worker's JS code and it can get control before the worker starts. We can see the DedicatedWorkerGlobalScope object on the call stack. But we need to know which nsIDOMWindow created the worker and that info does not seem to be in the DedicatedWorkerGlobalScope. This info will allow us to connect the Worker metadata with the correct web page.
See also Issue 2154: Firebug console and console methods innaccessible from Web Workers http://code.google.com/p/fbug/issues/detail?id=2154 The test case is also available via svn: http://code.google.com/p/fbug/source/browse/#svn/tests/issues/2154
Ideas on implementation: 1. The window raises even "workerCreated", one of the event properties is the worker object. 2. global observer event, subject is worker, topic "worker-created", data is the creator window. 3. DedicatedWorkerGlobalScope object is given a property 'window' which is the creating window. Any would work if the event comes before the first script is compiled.
I can find the DedicatedWorkerGlobalScope object: http://www.whatwg.org/specs/web-workers/current-work/#dedicatedworkerglobalscope in the stack frame when I break on the first PC of the worker function. But the object does not have a location property as promised by the parent interface http://www.whatwg.org/specs/web-workers/current-work/#workerglobalscope If this location was set correctly, we could decide this bug was fixed.
I poked around in the code but I did not find any |location| as a property. http://mxr.mozilla.org/mozilla-central/source/dom/interfaces/threads/nsIDOMWorkers.idl Maybe the mozilla code does not implement the whatwg spec completely?
Whiteboard: [firebug-p3]
Version: 1.9.1 Branch → Trunk
That was added after we had passed feature freeze for 3.5. We should implement it, bug 507930.
Depends on: 507930
(In reply to comment #5) > We should implement it Though I don't think that would really solve your problem. As you noted previously this method would fail to identify the correct nsIDOMWindow if the user had two tabs of the same page open.
Don't see way to nominated this for 1.9.3, sadly.... Ben, do workers currently have any sort of reference to their window?
Flags: blocking1.9.2?
(In reply to comment #7) > do workers currently have any sort of reference to their window? Yes, through their associated pool, and the thread service knows about these connections. Just out of curiosity, has the bigger question of whether jsd works off the main thread been investigated yet?
Rather than a static ref, how about an event; on the window thread just after Worker() starts. All we need is the window and the Scope object. When we see the Scope on the stack we match it to the window from the event. (In reply to comment #8) > Just out of curiosity, has the bigger question of whether jsd works off the > main thread been investigated yet? My guess is that jsd knows nothing about threads, including that it should not continue execution on through UI code once it gets called. So I guess it works off the worker thread just fine, up to the point where it steps on something in the UI and then yields. The crash will look like a UI failure. Had one of those: http://crash-stats.mozilla.com/report/index/4f343f65-7abb-4edc-93da-a4bd62090731 but we can't reproduce it. Just had another after setting a breakpoint on a worker.js (did not hit). The crash came after a long delay. But this one was right the tread event loop: http://crash-stats.mozilla.com/report/index/6e26b563-4b43-451f-b961-ba0122090802?p=1 (The fact that jsd can't run from the worker thread all the way to UI does not mean we can't do something for workers, just not breakpoints for now.)
jsd can work off the main thread, however the odds of anyone using it correctly are virtually nil. there's a patch to make that better, but i need some help to get reviews to get that landed. i'd suggest that realistically this is blocked on getting your code threadsafe. at least venkman's debugger was incestuous with dom windows. if you have a proper component which doesn't touch dom objects directly, and doesn't hold them, then it's possible that you aren't unsafe, but it's highly unlikely.
> When we see the Scope on the stack we match it to the window from the event. Doesn't that mean that you will be keeping the Scope alive in the meantime? At what point would you allow it to get garbage collected? I would rather not duplicate existing state in hashtables in jsd or firebug ui, since these out-of-band solutions don't have a very good idea of when it's ok to unroot objects or drop references to them (and the result is effective leaks).
(In reply to comment #11) > > When we see the Scope on the stack we match it to the window from the event. > > Doesn't that mean that you will be keeping the Scope alive in the meantime? At > what point would you allow it to get garbage collected? That is a problem with any solution for the debugger to track the Workers. This just means we need a matching event at the destruction of the Worker. That way we have a explicit way to know to drop the metadata.
(In reply to comment #10) ... > i'd suggest that realistically this is blocked on getting your code threadsafe. This particular bug report, requesting notification of the Worker creation/destruction, need not block on issues involving jsd threads. Separately I agree we can't get full debug support for a threaded js engine without additional work for thread safe action in the debuggers.
(In reply to comment #12) > This just means we need a matching event at the destruction of the Worker The worker will never be destroyed as long as you hold on to either the outer Worker object or the inner WorkerGlobalScope object.
(In reply to comment #14) > (In reply to comment #12) > > This just means we need a matching event at the destruction of the Worker > > The worker will never be destroyed as long as you hold on to either the outer > Worker object or the inner WorkerGlobalScope object. Ok sorry my terminology. How about: an event when the worker's closing flag is set true. Ideally we could know if the reason was killed, terminated, exit normal, exit error.
The worker has a "close" event that is close to what you're looking for, I think, though it fires on the worker's thread so you'd have to proxy something back to the UI somehow. The close event doesn't indicate why it's firing but we could probably expose something like that if it was really necessary. Close events were removed from the spec at the last minute because some implementations were having difficulty with them. They're only available on trunk (they barely missed the 3.5 freeze).
So something like: worker.addEventListener("close", foo, false); ? Is the event queue thread safe? Can I do: // on the UI thread, win is web page creating worker. var workerTag = new UID(); worker.addEventListener("close", function proxyCloseEvent(event) { // called on the worker thread, don't touch UI var ev = win.document.createEvent("Events"); ev.initEvent("worker-close", true, false); ev.tag = workerTag; win.dispatchEvent(ev); }, false);
(In reply to comment #17) > So something like: > worker.addEventListener("close", foo, false); > ? > > Is the event queue thread safe? Can I do: > > // on the UI thread, win is web page creating worker. > var workerTag = new UID(); > worker.addEventListener("close", function proxyCloseEvent(event) > { > // called on the worker thread, don't touch UI > var ev = win.document.createEvent("Events"); No, you can never ever touch a window or document object (or any other object from a main thread DOM scope) on any thread other than the main thread, whether you do anything that directly affects "UI" or not.
ok, so the close event on the worker thread is pretty worthless, since I can only use it on the thread that is now ending and I don't have anyway to get out other than the user's postMessage event, which we already agreed to avoid ?
Once you receive the close event you can't post messages to the main thread either as the worker object there might have been GC'd already. When I said you'd have to proxy something back to the main thread I meant outside the normal worker api somehow, like through a jsd hook or something.
but I think we can safely assume that jsd is not threadsafe, as in not developed with any idea about threads.
You can just use an XPCOM runnable to run some code on the main thread, no? That's the standard inter-thread communication mechanism right now...
Can I create the runnable on the main thread, close over the window object, then run it on the main thread from the event handler which is called on the worker thread? // on the UI thread, win is web page creating worker. var handleWorkerClose { run: function() { // stuff with window } }; worker.addEventListener("close", function proxyCloseEvent(event) { // called on the worker thread, got to switch threadManager.mainThread.dispatch(handleWorkerClose, 1); }, false);
No; that code you just posted will touch the window when it needs to run proxyCloseEvent (since the window is the root scope object for that function and hence where its security context comes from). The only safe way to pass functions around to different threads is to make sure they're created in a JS component (not a window) and do not close over any DOM objects directly.
Maybe I didn't make it clear but the "close" event only fires on the WorkerGlobalScope. In that code snippet you're adding a close event handler on the Worker object, not the WorkerGlobalScope. There's no way to accidentally mess up and do something unsafe because there is no way to get a window reference in the worker's script.
(In reply to comment #25) > ... There's no way to accidentally > mess up and do something unsafe because there is no way to get a window > reference in the worker's script. Fine, let's forget about it then. I need to get a window reference both on the Worker create and Worker close events on the UI thread. Otherwise I can't associate the Worker metadata with the window metadata. What are the options?
Flags: wanted1.9.2+
Flags: blocking1.9.2?
Flags: blocking1.9.2-
I am seeing getting more requests for this feature.
blocking2.0: --- → ?
Whiteboard: [firebug-p3] → [firebug-p2]
Just to be clear here. Simply fixing "Firebug needs some way to associate a Worker with the window that created it" doesn't really help right? Isn't all worker-related jsd callbacks happening off the main thread for example?
Correct, the association is not enough.We also need a way to block the worker thread, switch to the main thread, then call the jsd hook, switch back and unblock the worker. I guess this belongs in jsd's interrupt code. I suppose we should audit the control flow paths from js to jsd to avoid surprises.
I'm going to close this bug, because the js::dbg2 work will handle this problem.
Status: NEW → RESOLVED
blocking2.0: ? → ---
Closed: 14 years ago
Resolution: --- → INVALID
You need to log in before you can comment on or make changes to this bug.