1523262 - Debugging webassembly in workers

Status: RESOLVED FIXED

(DevTools :: Debugger, defect, P3)

Description

[Lars T Hansen [:lth]]

Attachment: 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".

Comment 1

[Anthony Jones (:ajones, :kentuckyfriedtakahe, :k17e)]

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.

Comment 2

[Jan Honza Odvarko [:Honza] (always need-info? me)]

(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

Comment 3

[Jan Honza Odvarko [:Honza] (always need-info? me)]

NI myself, so I have this on my dashboard.

Honza

Comment 4

[Jan Honza Odvarko [:Honza] (always need-info? me)]

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

Comment 5

[Jan Honza Odvarko [:Honza] (always need-info? me)]

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

Comment 6

[Lars T Hansen [:lth]]

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

Comment 7

[Brian Hackett [Laid off!]]

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.

Comment 8

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Thanks for the update Brian!

Honza

Comment 9

[Brian Hackett [Laid off!]]

Attachment: patch

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.

Comment 10

[Brian Hackett [Laid off!]]

Attachment: Part 1 - Inform worker we are observing wasm when attaching.

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.

Comment 11

[Brian Hackett [Laid off!]]

Attachment: Part 2 - Provide atob/btoa in worker debugger scope.

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.

Comment 12

[Logan Smyth [:loganfsmyth]]

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 :(

Comment 13

[Brian Hackett [Laid off!]]

(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 14

[Brian Hackett [Laid off!]]

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.

Comment 15

[Andrew McCreight [:mccr8]]

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.)

Comment 16

[Andrew Sutherland [:asuth] (he/him)]

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.

Comment 17

[Andrew McCreight [:mccr8]]

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

Comment 18

[Pulsebot]

Pushed by [e-mail]:
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.

Comment 19

[Raul Gurzau (:RaulG)]

https://hg.mozilla.org/mozilla-central/rev/7c6b5ef243de
https://hg.mozilla.org/mozilla-central/rev/4aa22c4f4c7b

Comment 20

[Lars T Hansen [:lth]]

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

Comment 21

[Jan Honza Odvarko [:Honza] (always need-info? me)]

(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