1747131 - Code for a JSM that is shown in the debugger is not what Firefox actually executes

Status: NEW

(DevTools :: Debugger, defect, P3)

Description

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Attachment: websocket-client.zip

Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:97.0) Gecko/20100101 Firefox/97.0 ID:20211219214917

I have an interesting situation where breakpoints do not work in the debugger. It happens when I change code within a JSM of the WebDriver BiDi implementation, and restart Firefox without rebuilding it - I'm using an artifact build. While the debugger in the MBT shows the right content, Firefox itself still executes the code from before.

Here the steps:

1. Apply the full set of patches from bug 1694389 to your local tree. Use the following command: moz-phab patch --apply-to 06c7b4e2d14b D134267.
2. Make sure to configure the tree for an artifact build, and do a re-build.
3. Download the websocket-client.zip and unzip its content
4. Start Firefox via mach run --remote-debugging-port --setpref="remote.log.level=Trace" --setpref="remote.log.truncate=false"
5. Open the client.html in another instance of Firefox
6. Enter localhost:9222 as address and click connect, followed by clicking the buttons for session.new and session.subscribe
7. Switch to the custom Firefox instance and open the MBT, search for browsingContext.jsm and set a valid breakpoint within the method #onMessageHandlerEvent()
8. Quit the custom Firefox
9. Open the file remote/webdriver-bidi/modules/root/browsingContext.html and change the line with the breakpoint by adding dump(\n*** some log\n\n); and a line break
10. Repeat steps 4 to 6
11. Open the MBT in the custom Firefox and inspect the breakpoint. It's now set for the line that includes the call to dump()
12. Open a new tab in the custom Firefox build.
13. Check that the debugger stops the execution on the dump line. Execute the next step and check the terminal for the expected output from the dump call => it cannot be seen
14. Close Firefox, do a re-built and repeat steps 10 - 13 => now the output is visible

That means without a rebuild Firefox works on outdated code while the debugger correctly shows the updated content of the JSM.

Alex, I hope that these steps are clear enough. If not please let me know.

Comment 1

[Julian Descottes [:jdescottes]]

Shorter STRs:

• open Firefox
• open and close context menu anywhere (just to ensure ContextMenuParent.jsm is loaded)
• open MBT
• set a breakpoint in ContextMenuParent.jsm at L23 (win.openContextMenu(message, browser, this);)
• close Firefox and MBT
• update ContextMenuParent.jsm to add dump("Some log\n"); at L23
• start Firefox
• start MBT
• open the context menu on Firefox

MBT will break and the debugger seems paused on dump("Some log\n");. But if you resume, the dump was not actually executed.

Comment 2

[Alexandre Poirot [:ochameau]]

Thanks a lot for the simplified STR!

It looks like it is related to ./mach build.
If you run ./mach build faster once (and just once) then, everytime you update the JSM, Firefox will use the latest content and debugger will show the right content.

So to replicate this STR, ensure doing a clobber and only use ./mach build once.

Then, regarding what's wrong on the debugger's end...
This actually seems to come from spidermonkey rather than DevTools server codebase.
We are fetching the JSM content from here:
https://searchfox.org/mozilla-central/rev/8d108a59d067ce37671090b0b1972ee8adfb7196/devtools/server/actors/source.js#228-231

      return {
        content: this.actualText(),
        contentType: "text/javascript",
      };

https://searchfox.org/mozilla-central/source/devtools/server/actors/source.js#241-254

  actualText() {
    const padding = this._source.startLine
      ? "\n".repeat(this._source.startLine - 1)
      : "";
    return padding + this._source.text;
  },

Ignore the padding story. The important take away here is that we rely on Debugger's Source.text attribute:
i.e. this c++ method https://searchfox.org/mozilla-central/rev/8d108a59d067ce37671090b0b1972ee8adfb7196/js/src/debugger/Source.cpp#241-258
This seems to be ultimately calling ReadSourceFromFilename:
https://searchfox.org/mozilla-central/rev/8d108a59d067ce37671090b0b1972ee8adfb7196/js/xpconnect/src/XPCJSRuntime.cpp#2753-2857
which probably fetches the source from the disk, instead of spidermonkey internal.

Unfortunately, we no longer have any official maintainer for this spidermonkey code.

Comment 3

[Julian Descottes [:jdescottes]]

If you run ./mach build faster once (and just once) then, everytime you update the JSM, Firefox will use the latest content

I was so confused by this! At some point I was unable to reproduce the bug and I had no idea what might have caused it. Thanks for looking into this.

Comment 4

[Julian Descottes [:jdescottes]]

Hi Yulia,

Debugger Source.cpp seems to read the source from the disk which is inconsistent with what Spidermonkey might be running. This means that our debugger can show a source which doesn't match the code executed by Firefox. See more details in comment #2. Is there any API we could use from SpiderMonkey to read the appropriate source text?

Thanks

Comment 5

[Yulia Startsev [:yulia]]

It sounds like what is happening is we are running the cached code but fetching the source for debugging? Hm, This sounds a bit tricky to solve and we don't have an API that I know of, but I will ask around the spidermonkey team.

Comment 6

[Jan de Mooij [:jandem]]

SpiderMonkey likely doesn't have access to the source code it's executing in this case, just a compiled/cached representation of it. It's falling back to the load-source hook to fetch the source code, but this file has changed in the meantime.

Does running with the MOZ_PURGE_CACHES=1 environment variable help at all?

Comment 7

[Julian Descottes [:jdescottes]]

Thanks for looking into this!

I can confirm that using MOZ_PURGE_CACHES=1 with the STRs from comment #1 avoids the issue.
Now I don't know if that's something we can reuse to fix the problem. The browser toolbox is opened after the target Firefox has started, so we can't really force MOZ_PURGE_CACHES early enough.

I guess we could print a warning in the Console when the Browser Toolbox is opened and this environment variable is not set in the target Firefox.

Comment 8

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Hm, I wonder if we could at least set this environment variable when any mach command is used with the --jsdebugger argument?

Comment 9

[Julian Descottes [:jdescottes]]

Ni: to add a comment

Comment 10

[Julian Descottes [:jdescottes]]

Reviewed during triage, team agreed that we cannot much more than what is proposed at comment #7 and comment #8, and we'll happily take patches for this.