Debugging webassembly in workers

RESOLVED FIXED in Firefox 67

Status

defect
P3
normal
RESOLVED FIXED
6 months ago
3 months ago

People

(Reporter: lth, Assigned: bhackett)

Tracking

(Blocks 1 bug)

unspecified
Firefox 67
Desktop
All
Dependency tree / graph

Firefox Tracking Flags

(firefox67 fixed)

Details

Attachments

(4 attachments)

Posted file hello.tar

There is no debugger functionality for WebAssembly in workers, yet this will be an important use case for wasm in the future (we're seeing it already).

I'm attaching a simple test. Unpack this and run server.py to start a server on localhost:8000, then load localhost:8000/hello.html in a new FF tab with the developer console open. It will create a worker from hello-worker.js, which will load hello-worker.wasm.

Once the debugger has an inspector focussed on the worker in this test, there are two files, a JS file and a wasm file. The wasm file can't be shown, an error in the text pane of the inspector says "Please refresh to debug this module". There are several problems with this:

  • Most important, if I reload the page I get a different worker instance, so no breakpoints I've set in the inspector for the worker that was created during the first load appear to be respected by the reload.

  • Second, whether the worker is even available to debug from the main debugger window seems to be a little random, the line in the UI with the worker on it disappears as soon as I interact with the frame that inspects the worker, and it does not come back when I close the worker inspector. (That is, load the page with the debugger open, click on "hello-worker.js" under Workers, and when that inspector launches, click on hello-worker.js in the worker inspector sidebar. Now the section for "workers" in the main debugger window is gone.)

  • Least important, the terminology in the error message should probably be "reload" and not "refresh".

Priority: -- → P3

Honza - is I'd like to get an indication of where this sits in your 1H priorities. I'm worried that the lack of worker support will become an increasing expense for my team.

Flags: needinfo?(odvarko)

(In reply to Anthony Jones (:ajones, :kentuckyfriedtakahe, :k17e) from comment #1)

Honza - is I'd like to get an indication of where this sits in your 1H priorities. I'm worried that the lack of worker support will become an increasing expense for my team.

Hey Anthony! There is no plan for working on WASM in H1, but perhaps we could schedule something for H2? Harald?

Honza

Flags: needinfo?(odvarko) → needinfo?(hkirschner)

NI myself, so I have this on my dashboard.

Honza

Flags: needinfo?(odvarko)

Just confirmation that I was able to reproduce this issue on my machine.

Brian, could your work on workers improve the situation here?
Is this more related to WASM or worker debugging?

Honza

Flags: needinfo?(bhackett1024)

Perhaps this could be related to the fact that the Debugger is attached too late? Would pausing the worker imediatelly, attaching the debugger and resume help to solve this?

Honza

Pausing a worker (any worker) on startup would be tremendously helpful in general, actually :)

Currently we don't have a way to pause workers on startup, but bug 1527203 has patches to fix this. That bug is geared towards being able to set breakpoints in workers on startup (fixing the first point in comment 0), but the infrastructure added there would make it pretty easy to add functionality for pausing all workers on startup.

All of this is gated on the new windowless worker UI (where worker threads are inspected from the same toolbox which debugs the main thread), which is off by default but can be enabled with the devtools.debugger.features.windowless-workers pref.

Testing with this pref, the debugger is still broken --- I get an error when opening hello-worker.js. I'll look into this in the next day or two and see how far I can get with debugging the worker with the changes in bug 1527203. I don't know whether the main problems will be with the worker support or wasm support, as I don't have any familiarity with how the debugger deals with the latter.

Thanks for the update Brian!

Honza

Depends on: 1527203
Flags: needinfo?(odvarko)
Posted patch patchSplinter Review

When I apply this patch on top of bug 1527203, the worker's wasm file is loaded consistently in the debugger after reloading. I haven't done any additional testing, but breakpoints should be hit in the worker as expected after bug 1527203 lands.

I don't know much at all about the debugger's wasm support, but in general things that work when debugging the main thread should work when debugging a worker thread. Worker support is still pretty rough but we want to get things ironed out in the near future, so please file any issues you run into and I'll try to look at them quickly.

Assignee: nobody → bhackett1024
Flags: needinfo?(bhackett1024)

Unless the debugger is explicitly marked as observing wasm sources, those sources will not save the information needed to show them in the debugger. This patch sets this information in the debugger when attaching, instead of waiting for a reconfigure as is currently required. After bug 1527203 lands, this logic will run before any content in the worker has been processed.

Attachment #9045843 - Flags: review?(lsmyth)

Showing the wasm source requires creating an ArrayBuffer actor in the worker (I'm not sure why, but this isn't too surprising), which requires btoa to be defined in the global scope. This isn't currently the case in the scope in which debugger code in the worker runs.

Attachment #9045844 - Flags: review?(lsmyth)
Attachment #9045844 - Flags: review?(lsmyth) → review+
Comment on attachment 9045843 [details] [diff] [review]
Part 1 - Inform worker we are observing wasm when attaching.

Review of attachment 9045843 [details] [diff] [review]:
-----------------------------------------------------------------

This option only mentioning ASMjs and not WASM seems unfortunate :(
Attachment #9045843 - Flags: review?(lsmyth) → review+

(In reply to Logan Smyth [:loganfsmyth] from comment #12)

Comment on attachment 9045843 [details] [diff] [review]
Part 1 - Inform worker we are observing wasm when attaching.

Review of attachment 9045843 [details] [diff] [review]:

This option only mentioning ASMjs and not WASM seems unfortunate :(

Yeah, the option should be renamed but this is the name used when the main thread is reconfigured.

Comment on attachment 9045844 [details] [diff] [review]
Part 2 - Provide atob/btoa in worker debugger scope.

This patch touches webidl and also needs review from a DOM peer.
Attachment #9045844 - Flags: review+ → review?(continuation)
Comment on attachment 9045844 [details] [diff] [review]
Part 2 - Provide atob/btoa in worker debugger scope.

Review of attachment 9045844 [details] [diff] [review]:
-----------------------------------------------------------------

asuth should review this. He might know of some existing way to get atob and btoa onto worker scopes. I know for main process scopes we have a few ways. r=me for the WebIDL changes if he's okay with the patch (I don't know if DOM worker peerage helps with the WebIDL commit hooks or not.)
Attachment #9045844 - Flags: review?(continuation)
Attachment #9045844 - Flags: review?(bugmail)
Attachment #9045844 - Flags: review+
Comment on attachment 9045844 [details] [diff] [review]
Part 2 - Provide atob/btoa in worker debugger scope.

Review of attachment 9045844 [details] [diff] [review]:
-----------------------------------------------------------------

What a weird scope!  The WebIDL hooks hate me, and :baku may have other insight, so redirecting to :baku, but this looks right given that the WorkerDebuggerGlobalScope is its own thing that doesn't chain up to our normal worker global base interfaces.
Attachment #9045844 - Flags: review?(bugmail) → review?(amarchesini)
Attachment #9045844 - Flags: review?(amarchesini) → review+
Attachment #9045844 - Flags: review+

baku is a DOM peer, so no need for my nominal r+ now.

Pushed by bhackett@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/7c6b5ef243de
Part 1 - Inform worker we are observing wasm when attaching, r=lsmyth.
https://hg.mozilla.org/integration/mozilla-inbound/rev/4aa22c4f4c7b
Part 2 - Provide atob/btoa in worker debugger scope, r=lsmyth,baku.
Status: NEW → RESOLVED
Closed: 5 months ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 67

I've been testing this on m.twitch.tv and it appears to work well. Thanks!

(In reply to Lars T Hansen [:lth] from comment #20)

I've been testing this on m.twitch.tv and it appears to work well. Thanks!

Great, thanks for the update!
Honza

Flags: needinfo?(hkirschner)
You need to log in before you can comment on or make changes to this bug.