Skip to content

Add shutdown watchdog for stalled Electron workers - #5985

Merged
michelinewu merged 9 commits into
stagingfrom
shutdown-watchdog
Jul 27, 2026
Merged

michelinewu merged 9 commits into
stagingfrom
shutdown-watchdog

Conversation

@aleksandr-voitenko

@aleksandr-voitenko aleksandr-voitenko commented Jun 18, 2026

Copy link
Copy Markdown
Collaborator

Adds a coordinated, bounded shutdown flow across Electron’s main process and worker renderer. The main process waits up to 10 seconds for acknowledgement, 30 seconds for worker teardown, and 10 seconds for Electron to exit after completion, force-exiting when a phase stalls.

Worker shutdown now persists local scene and session state before destructive OBS teardown, continues remaining steps after individual failures, bounds analytics flushing to three seconds, and reports whether shutdown completed cleanly. File flushing now supports concurrent shutdown callers and writes that race an in-flight flush.

If a second instance starts during shutdown, its arguments are preserved and one relaunch is scheduled; the active watchdog continues to bound the original process’s exit. Scene-collection cloud synchronization is deferred to startup so network stalls cannot block shutdown.

@aleksandr-voitenko aleksandr-voitenko changed the title Add shutdown watchdog for stalled Electron workers [WIP] Add shutdown watchdog for stalled Electron workers Jun 18, 2026
@bundlemon

bundlemon Bot commented Jun 18, 2026

Copy link
Copy Markdown

BundleMon

Files added (4)
Status Path Size Limits
renderer.(hash).js
+10.36MB -
vendors~renderer.(hash).js
+4.67MB -
updater.js
+115.29KB -
guest-api.js
+40.23KB -

Total files change +15.18MB

Final result: ✅

View report in BundleMon website ➡️


Current branch size history

@aleksandr-voitenko aleksandr-voitenko changed the title [WIP] Add shutdown watchdog for stalled Electron workers Add shutdown watchdog for stalled Electron workers Jun 18, 2026
@michelinewu michelinewu added the P0 highest priority to merge from Asana label Jul 22, 2026
@sandboxcoder
sandboxcoder requested a review from Copilot July 23, 2026 16:53

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds a coordinated, time-bounded shutdown watchdog across Electron main + worker, and restructures worker shutdown into ordered phases that preserve local state before OBS teardown while keeping best-effort work bounded.

Changes:

  • Introduces a main-process shutdown coordinator with acknowledgement / completion / exit watchdog timers, plus controlled relaunch scheduling during shutdown.
  • Adds a worker shutdown runner with ordered persistence → teardown → best-effort phases, step-level failure reporting, and bounded analytics flushing.
  • Improves shutdown durability/robustness via scene-collection persistence split, stricter crash-reporter state writes, and a flushAll implementation that supports concurrent callers and racing writes.

Reviewed changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
test/regular/worker-shutdown.ts Adds unit-style coverage for ordered worker shutdown phases, failures, and time-bounded best-effort steps.
test/regular/file-manager-shutdown.ts Adds tests for concurrent/racing flushAll() behavior and retry exhaustion.
test/regular/app-shutdown.ts Adds tests validating shutdown watchdog timing, idempotency, and relaunch scheduling semantics.
main.js Integrates shutdown coordinator into main-process shutdown flow, adds force-exit behavior, and preserves argv for relaunch during shutdown.
electron-builder/base.config.js Ensures the new shutdown coordinator module is packaged into builds.
app/util/worker-shutdown.ts New generic shutdown-step runner with reporting, per-step timeouts, and clean-exit marking.
app/util/shutdown-coordinator.js New main-process watchdog state machine coordinating shutdown acknowledgement/completion/exit phases.
app/services/usage-statistics.ts Adds optional AbortSignal support so analytics flushing can be bounded during shutdown.
app/services/streaming/streaming.ts Accumulates shutdown teardown failures and throws a consolidated error after attempting all destroys.
app/services/scene-collections/scene-collections.ts Splits persistence for shutdown from teardown, and removes cloud sync from shutdown path.
app/services/file-manager.ts Reworks flushAll() to coordinate concurrent callers and handle racing rewrites/new files during shutdown.
app/services/crash-reporter.ts Makes shutdown state persistence failures explicit by returning success and throwing on failure.
app/services/app/app.ts Refactors worker shutdown into immediate ingress-stop + phased shutdown plan executed via the new shutdown runner.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread app/util/shutdown-coordinator.js
Comment thread main.js
if (!window.isDestroyed()) window.destroy();
});

app.exit(0);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is 0 intentional?
it is not exactly clean exit, but not a full failure too.

Comment thread main.js
shutdownCoordinator.acknowledgeShutdown();
});

ipcMain.on('shutdownComplete', () => {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

as we now send additional information with this message.

          electron.ipcRenderer.send('shutdownComplete', {
            clean: report.clean,
            failedSteps: report.failures.map(failure => failure.name),
          });

probably be useful to log it?

ipcMain.on('shutdownComplete', (event, payload) => {
  const { clean, failedSteps } = payload;

@michelinewu
michelinewu merged commit 03d2bff into staging Jul 27, 2026
8 of 13 checks passed
@michelinewu
michelinewu deleted the shutdown-watchdog branch July 27, 2026 18:19
sandboxcoder added a commit that referenced this pull request Aug 17, 2026
* Add shutdown watchdog for stalled Electron workers

* Comments update

* Trivial whitespace fix

* Fix shutdown watchdog safety issues

* Adjust shutdown tests

* Restore staging release versions

* wrap logging in try/catch

---------

Co-authored-by: Richard Osborne <rosborne255@yahoo.com>
(cherry picked from commit 03d2bff)
sandboxcoder added a commit that referenced this pull request Aug 21, 2026
* Add shutdown watchdog for stalled Electron workers

* Comments update

* Trivial whitespace fix

* Fix shutdown watchdog safety issues

* Adjust shutdown tests

* Restore staging release versions

* wrap logging in try/catch

---------

Co-authored-by: Richard Osborne <rosborne255@yahoo.com>
(cherry picked from commit 03d2bff)
sandboxcoder added a commit that referenced this pull request Aug 21, 2026
* Add shutdown watchdog for stalled Electron workers

* Comments update

* Trivial whitespace fix

* Fix shutdown watchdog safety issues

* Adjust shutdown tests

* Restore staging release versions

* wrap logging in try/catch

---------

Co-authored-by: Richard Osborne <rosborne255@yahoo.com>
(cherry picked from commit 03d2bff)
sandboxcoder added a commit that referenced this pull request Aug 24, 2026
* Add shutdown watchdog for stalled Electron workers

* Comments update

* Trivial whitespace fix

* Fix shutdown watchdog safety issues

* Adjust shutdown tests

* Restore staging release versions

* wrap logging in try/catch

---------

Co-authored-by: Richard Osborne <rosborne255@yahoo.com>
(cherry picked from commit 03d2bff)
michelinewu pushed a commit that referenced this pull request Aug 24, 2026
…nboarding, and cherry-pick other test fixes (#6080)

* Update for new onboarding flow

onboarding.ts - fix false positive by removing the
invalid early return & update the Cases that were
not running to use the new onboarding flow.

* Fix RxJS unsubscribe crash and missing React keys in onboarding

Six components returned `sub.unsubscribe` directly as their useEffect
cleanup. React invokes cleanups as bare function calls, so the method
was detached from its Subscriber and reading `this.closed` threw:

  Uncaught TypeError: Cannot read properties of null (reading 'closed')
      at Subscriber.unsubscribe
      at flushPassiveEffectsImpl

Because unsubscribe() threw before doing any work, these subscriptions
were never actually torn down either. Return `() => sub.unsubscribe()`
so the call keeps its receiver.

Affected: Themes, Ultra, MacPermissions, EditTransform,
PlatformAppPopOut, Troubleshooter.

Also add the two missing `key` props in Themes.tsx -- on the PreviewCard
carousel children, and on the thumbnail column inside PreviewCard. Both
were logging "Each child in a list should have a unique key prop" at
error level, which fails the log check.

Remove the darwin-gated filter for this error from the webdriver log
check. It was hiding the bug above on macOS, and because it set
`ignoringErrors = true` it also swallowed every subsequent error record
until the next non-error line, masking unrelated failures.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
(cherry picked from commit f464ff6)

* Add shutdown watchdog for stalled Electron workers (#5985)

* Add shutdown watchdog for stalled Electron workers

* Comments update

* Trivial whitespace fix

* Fix shutdown watchdog safety issues

* Adjust shutdown tests

* Restore staging release versions

* wrap logging in try/catch

---------

Co-authored-by: Richard Osborne <rosborne255@yahoo.com>
(cherry picked from commit 03d2bff)

* [MacOS] Verify Continue button exists before clicking

* Kill stale ChromeDriver process (#6057)

Issue #1:
Fix for issue I encountered when I tried to run a test which failed
because a stale chromedriver process blocked the port.

Error spew:
  Rejected promise returned by test. Reason:

  Error {
    message: `Failed to create session.␊
    socket hang up`,
  }

  › socket hang up
  › startWebDriverSession (node_modules/webdriver/build/utils.js:73:15)
  › Function.newSession (node_modules/webdriver/build/index.js:46:45)
  › remote (node_modules/webdriverio/build/index.js:77:22)

Verified fix by:
Start a fake stale chromedriver in the background:
./node_modules/electron-chromedriver/bin/chromedriver --port=4444 &

On Mac, I run:
lsof -ti :4444
On Windows:
netstat -ano | findstr :4444

Run the test:
yarn test:file test-dist/test/regular/api/scene-collections.js

It will now terminate the process that owns the port allowing a new
ChromeDriver instance to start listening on the port

Issue #2:
api-client.js [MacOS code path] - use socket.end() to fix test failure
for scene-collections.ts that I saw on MacOS.

Error spew:
  [2026-07-28T21:36:31.135Z] [error] [worker] - Uncaught Error: read ECONNRESET Error: read ECONNRESET
  [2026-07-28T21:36:31.135Z] [error] [worker] -     at __node_internal_captureLargerStackTrace (node:internal/errors:497:5)
  [2026-07-28T21:36:31.135Z] [error] [worker] -     at __node_internal_errnoException (node:internal/errors:624:12)
  [2026-07-28T21:36:31.135Z] [error] [worker] -     at TCP.onStreamRead (node:internal/stream_base_commons:217:20) {
  [2026-07-28T21:36:31.135Z] [error] [worker] -   errno: -54,
  [2026-07-28T21:36:31.135Z] [error] [worker] -   code: 'ECONNRESET',
  [2026-07-28T21:36:31.135Z] [error] [worker] -   syscall: 'read'
  [2026-07-28T21:36:31.135Z] [error] [worker] - }

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-authored-by: Aleksandr Voitenko <aleksandr.voitenko1986@gmail.com>
michelinewu pushed a commit that referenced this pull request Aug 24, 2026
* Update for new onboarding flow

onboarding.ts - fix false positive by removing the
invalid early return & update the Cases that were
not running to use the new onboarding flow.

* Fix RxJS unsubscribe crash and missing React keys in onboarding

Six components returned `sub.unsubscribe` directly as their useEffect
cleanup. React invokes cleanups as bare function calls, so the method
was detached from its Subscriber and reading `this.closed` threw:

  Uncaught TypeError: Cannot read properties of null (reading 'closed')
      at Subscriber.unsubscribe
      at flushPassiveEffectsImpl

Because unsubscribe() threw before doing any work, these subscriptions
were never actually torn down either. Return `() => sub.unsubscribe()`
so the call keeps its receiver.

Affected: Themes, Ultra, MacPermissions, EditTransform,
PlatformAppPopOut, Troubleshooter.

Also add the two missing `key` props in Themes.tsx -- on the PreviewCard
carousel children, and on the thumbnail column inside PreviewCard. Both
were logging "Each child in a list should have a unique key prop" at
error level, which fails the log check.

Remove the darwin-gated filter for this error from the webdriver log
check. It was hiding the bug above on macOS, and because it set
`ignoringErrors = true` it also swallowed every subsequent error record
until the next non-error line, masking unrelated failures.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
(cherry picked from commit f464ff6)

* Add shutdown watchdog for stalled Electron workers (#5985)

* Add shutdown watchdog for stalled Electron workers

* Comments update

* Trivial whitespace fix

* Fix shutdown watchdog safety issues

* Adjust shutdown tests

* Restore staging release versions

* wrap logging in try/catch

---------

Co-authored-by: Richard Osborne <rosborne255@yahoo.com>
(cherry picked from commit 03d2bff)

* [MacOS] Verify Continue button exists before clicking

* Kill stale ChromeDriver process (#6057)

Issue #1:
Fix for issue I encountered when I tried to run a test which failed
because a stale chromedriver process blocked the port.

Error spew:
  Rejected promise returned by test. Reason:

  Error {
    message: `Failed to create session.␊
    socket hang up`,
  }

  › socket hang up
  › startWebDriverSession (node_modules/webdriver/build/utils.js:73:15)
  › Function.newSession (node_modules/webdriver/build/index.js:46:45)
  › remote (node_modules/webdriverio/build/index.js:77:22)

Verified fix by:
Start a fake stale chromedriver in the background:
./node_modules/electron-chromedriver/bin/chromedriver --port=4444 &

On Mac, I run:
lsof -ti :4444
On Windows:
netstat -ano | findstr :4444

Run the test:
yarn test:file test-dist/test/regular/api/scene-collections.js

It will now terminate the process that owns the port allowing a new
ChromeDriver instance to start listening on the port

Issue #2:
api-client.js [MacOS code path] - use socket.end() to fix test failure
for scene-collections.ts that I saw on MacOS.

Error spew:
  [2026-07-28T21:36:31.135Z] [error] [worker] - Uncaught Error: read ECONNRESET Error: read ECONNRESET
  [2026-07-28T21:36:31.135Z] [error] [worker] -     at __node_internal_captureLargerStackTrace (node:internal/errors:497:5)
  [2026-07-28T21:36:31.135Z] [error] [worker] -     at __node_internal_errnoException (node:internal/errors:624:12)
  [2026-07-28T21:36:31.135Z] [error] [worker] -     at TCP.onStreamRead (node:internal/stream_base_commons:217:20) {
  [2026-07-28T21:36:31.135Z] [error] [worker] -   errno: -54,
  [2026-07-28T21:36:31.135Z] [error] [worker] -   code: 'ECONNRESET',
  [2026-07-28T21:36:31.135Z] [error] [worker] -   syscall: 'read'
  [2026-07-28T21:36:31.135Z] [error] [worker] - }

* Fix test/sources.ts 'Source events' test (for MacOS) (#6062)

call fetchNextEvent() before the triggering action so the Subject
subscriber is already active when the event arrives during
deasync.loopWhile. When the Promise later resolves, await on an
already-resolved Promise returns immediately.

* Fix waitForElectronInstancesExist deadline and log elapsed time

The 10s budget was enforced by subtracting the poll interval once per
pass, but each pass also spends real time inside getElectronInstances()
-- tasklist costs a few hundred ms on Windows -- so the nominal 10s ran
closer to 13-15s of wall clock. Track a start timestamp and compare
against that instead, so the constant means what it says.

Report the measured duration for the timeout error

Document why the budget is deliberately short. A stalled shutdown is
unbounded rather than slow -- the worker can block in a synchronous
native OBS call, and the main-process watchdog added in ef1865e cannot
fire while main is blocked as well, so nothing rescues it. Raising the
budget only delays an identical failure.

Behaviour is otherwise unchanged. Also reindents the trailing if block,
which Prettier had never been run over.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* Update sources.ts to run on MacOS

* Add partial support for MacOS.

* fix 'Adding and removing a Color Correction' filter

* fix 'Adding and removing a Noise Suppression filter' test

* Update test to validate version 2 filter properties

* fix(api-client): handle TCP packet fragmentation and deasync promise race

Three bugs fixed in the test API client's socket communication:

1. Buffered newline-delimited message reassembly: raw socket `data` events
   deliver arbitrary byte chunks, so large JSON responses (e.g. after
   installOverlay) were split across events, causing JSON.parse to throw
   on incomplete fragments. Incoming bytes are now accumulated in
   `receiveBuffer` and flushed line-by-line via StringDecoder.

2. requestSync multi-message response: the one-off sync socket could
   accumulate more than one newline-delimited message (e.g. an event
   followed by the actual reply). JSON.parse on the concatenated string
   failed with "Unexpected token {". Fixed by parsing only the first
   non-empty line of the response buffer.

3. Early PROMISE resolution race with deasync: when a service method
   resolves immediately (e.g. SceneCollectionsService.delete on a
   non-active collection), the PROMISE resolution event arrives on the
   persistent socket while deasync.loopWhile is still driving the event
   loop inside requestSync — before handleRequest has registered the
   promise handler. The silent drop caused a 20s timeout. Fixed by
   buffering early resolutions in `earlyPromiseResults` and replaying
   them the moment the promise is registered.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* Clear earlyPromiseResults on reconnect to prevent stale entries

* fix obs-importer.ts (new onboarding flow)

* Update 'Adding and removing a LUT filter' test for v2 component

* Add test for Passthrough Alpha checkbox

* removed extra comment

* fix(api-client): match requestSync response by id

Avoids returning the wrong payload if an event or unrelated message
shares the same TCP chunk as the actual reply.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* socket.onclose handler

* remove this code before the StringDecoder only matters mid-stream

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-authored-by: Aleksandr Voitenko <aleksandr.voitenko1986@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug P0 highest priority to merge from Asana performance

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants