1982950 - MOZ_CRASH(TimeoutExecutor not thread-safe) due to pending timer in subprocess_worker.win.js

Status: RESOLVED FIXED

(Toolkit :: Async Tooling, defect, P1)

Description

[Rob Wu [:robwu]]

The crash associated with bug 1982170 went up after the patch to bug 1979546 landed. This means that in the new implementation, a timer is scheduled to run at the next macrotask (queued here), but never gets a chance to run, while this was not happening in the previous implementation.

Although it is easy to fix the test-only crashes (I've attached a patch to bug 1982170), I'd like to get make sure that the cause of this behavioral change is clear, and acknowledged or fixed as needed.

Comment 1

[BugBot [:suhaib / :marco]]

Set release status flags based on info from the regressing bug 1979546

Comment 2

[Rob Wu [:robwu]]

Attachment: Bug 1982950 - Stop polling as soon as process exits

The patch to bug 1979546 refactored the logic that awaits the process
termination. The logic that processes process termination should stop
the poll loop() via updatePollEvents(). This did not happen, because
updatePollEvents() considered a process alive if it is in
io.processes, but the updatePollEvents() was not called when
cleanupProcess cleared the last reference.

Consequently, the poll loop continued forever if the process's pipes
closed before the process.

This patch fixes the issue and adds unit tests. As part of the unit
tests, we must wait for stdout to be closed. That was broken on Windows
due to a long-standing pre-existing bug: readBuffer() used
libc.winLastError to check for errors, but that property is never set
because it is on ctypes.winLastError. This patch also fixes that.

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/bdbd7ec52626
https://hg.mozilla.org/integration/autoland/rev/45c0992096bb
Stop polling as soon as process exits r=gstoll

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/3bb9b9dc739d
https://hg.mozilla.org/integration/autoland/rev/6b5b913098c9
Revert "Bug 1982950 - Stop polling as soon as process exits r=gstoll" for causing multiple failures

Comment 5

[Cristina Horotan [:chorotan]]

Backed out for causing multiple failures

Backout link

Push with failures

Failure log test_stub_installer.js
Failure log browser_console_to_mozlog.js
Failure log test_backgroundtask_deletes_profile.js
Failure log browser_sponsor_protection.js
Failure log test_ext_native_messaging_concurrent.js

Comment 6

[Rob Wu [:robwu]]

Unfortunately fixing the long-standing issue with the use of the non-existing libc.winLastError causes failures. Since it is not critical to fixing the regression I'm moving that to bug 1983138.

Even without the readString() part of the test (disabled because it depends on the fix to bug 1983138), the test still provides the desired test coverage: The most important part of the test is "Not polling when last process has exited" check, and I confirmed that it fails without the fix, and passes with the fix.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/551c679d82f8
https://hg.mozilla.org/integration/autoland/rev/1387ca288bc3
Stop polling as soon as process exits r=gstoll

Comment 8

[Norisz Fay [:noriszfay]]

https://hg.mozilla.org/mozilla-central/rev/1387ca288bc3

Comment 9

[Rob Wu [:robwu]]

Attachment: Bug 1982950 - Stop polling as soon as process exits

The patch to bug 1979546 refactored the logic that awaits the process
termination. The logic that processes process termination should stop
the poll loop() via updatePollEvents(). This did not happen, because
updatePollEvents() considered a process alive if it is in
io.processes, but the updatePollEvents() was not called when
cleanupProcess cleared the last reference.

Consequently, the poll loop continued forever if the process's pipes
closed before the process.

Original Revision: https://phabricator.services.mozilla.com/D261060

Comment 10

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-esr140/rev/f8c237a193f7