Skip to content

fix: delay on exit#8871

Merged
NickKhalow merged 24 commits into
devfrom
chore/delay-on-exit-fix
Jun 5, 2026
Merged

fix: delay on exit#8871
NickKhalow merged 24 commits into
devfrom
chore/delay-on-exit-fix

Conversation

@NickKhalow

@NickKhalow NickKhalow commented May 28, 2026

Copy link
Copy Markdown
Contributor

Why

After profiling with native tooling (sample, wpr, xperf), the root cause of the slow Explorer shutdown was identified: the bottleneck is Unity's native teardown, not managed code. The delay is most pronounced in Genesis Plaza, likely due to the high GameObject count. A hypothesis supported by a test run where manually destroying all GameObjects shifted the cost into managed cleanup time.

Benchmarks (with native exit stopwatch)

Scenario Native shutdown (ms) Managed cleanup (ms)
Genesis Plaza (run 1) 8,159 1,066
Genesis Plaza (run 2) 11,383 1,177
World (run 1) 1,772 1,048
World (run 2) 1,564 925
GP → World (run 1) 8,231 1,218
GP → World (run 2) 7,937 1,109
GP → World (manual OnDestroy) 10,611 4,725

Key takeaway: Native shutdown takes 8–11s in Genesis Plaza vs 1.5–1.8s in Worlds. Managed cleanup is consistently ~1s. Manually calling OnDestroy on all GameObjects moved cost into managed time but did not reduce total exit time — confirming the problem is in Unity's native object destruction.

Chosen Strategy

  1. Ensure graceful shutdown of all critical subsystems first (disk I/O, Sentry session flush, crash detector flag, player prefs).
  2. Forcefully terminate the process via TerminateProcess (Win) / kill SIGKILL (macOS) immediately after, bypassing Unity's native teardown entirely.

Open item: The root cause of why Unity's native teardown takes up to 15 seconds is still unknown and will be investigated further with Unity support.

image

Red - user clicks Exit
Yellow - Runtime::Shutdown (by Unity)
image

GC_end_stubborn_change looks suspicious because it's being called for a long time after calling the exit
image

image

What does this PR change?

This PR overhauls the application exit flow to eliminate the delay/hang users experience when closing the Explorer on Windows and macOS. Instead of relying on Unity's native teardown, the exit path now follows a controlled sequence:

  1. Centralized graceful cleanup — All critical teardown work (CrashDetector flag clearing, Sentry session flush, DCLPlayerPrefs save) is executed synchronously via ExitUtils cleanup candidates, in a deterministic order, before any quit signal is dispatched.
  2. Hard process termination as a backstop — After Unity's Application.Quit() fires all quitting subscribers, a final subscriber calls TerminateProcess (Windows) or kill(SIGKILL) (macOS) to guarantee the process exits immediately, preventing Unity's native shutdown from hanging.
  3. Native exit stopwatch — An optional external Windows utility (dcl_exit_stopwatch.exe) can be launched via the --native-shutdown-stopwatch flag to measure the real wall-clock time from exit request to process death, for diagnostics.

Key changes

  • ExitUtils.cs — Added TryTerminateSelf() (hard kill via P/Invoke), Configure() for runtime flags, main-thread guard on Exit(), DCLPlayerPrefs.SaveSync() flush in the exit sequence, and reorganized the Patch class to always keep TryTerminateSelf as the very last Application.quitting subscriber.
  • CrashDetector.cs — Converted from MonoBehaviour (with DontDestroyOnLoad GameObject) to a static class that registers with ExitUtils.RegisterCleanUpCandidate. Prefs save is now centralized in ExitUtils.
  • SentryReportHandler.cs — Registers a cleanup candidate that calls SentrySdk.EndSession() + SentrySdk.Flush(2s) to ensure the final session envelope is sent before termination.
  • MainSceneLoader.cs — Added canShutdown guard to prevent double-shutdown and non-main-thread calls. Registers itself as a cleanup candidate and configures ExitUtils with app arg flags.
  • DynamicWorldContainer.cs — Added --no-livekit-mode flag support: when set, uses NullRoomHub.INSTANCE to bypass all LiveKit room connections.
  • AppArgsFlags.cs — Added three new flags: no-livekit-mode, native-shutdown-stopwatch, soft-shutdown.
  • dcl_exit_stopwatch.c — New native Windows utility that attaches to the Explorer process by PID, waits for exit, and writes the elapsed time (ms) to a log file.

Test Instructions

Steps (standard run):

metaforge explorer run 8871

Expected result:

  • Explorer should close within 2-3 seconds when the user clicks the close button — no visible delay or hang for dozens of seconds.
  • DCLPlayerPrefs and crash detector state should persist correctly across sessions.
  • Sentry session should end cleanly (verify in Sentry dashboard).

Steps (with native stopwatch):

metaforge explorer run 8871 -- --native-shutdown-stopwatch

Expected result:

  • An exit_stopwatch_<timestamp>.log file should appear in the persistent data folder after exit, containing the exit duration in milliseconds.

Quality Checklist

  • Changes have been tested locally
  • Performance impact has been considered

@NickKhalow NickKhalow self-assigned this May 28, 2026
@NickKhalow NickKhalow added the force-build Used to trigger a build on draft PR label May 28, 2026
@github-actions

github-actions Bot commented May 28, 2026

Copy link
Copy Markdown
Contributor

@github-actions

Copy link
Copy Markdown
Contributor

badge

New build in progress, come back later!

@NickKhalow NickKhalow changed the base branch from dev to main May 28, 2026 16:31
@NickKhalow NickKhalow added the clean-build Used to trigger clean build on PR label May 28, 2026
@m3taphysics

Copy link
Copy Markdown
Collaborator

PR #8871, run #26605462244

Builds: Windows change, Windows baseline, macOS change, macOS baseline

Framework 13 i7

Metric Change Baseline Delta Improvement
Samples 2699 2700
CPU average 33.3 ms 33.3 ms 0.0 ms -0.0%
CPU 1% worst 34.0 ms 33.7 ms 0.3 ms -1.0%
CPU 0.1% worst 38.1 ms 34.7 ms 3.4 ms -9.8% 🔴
GPU average 7.3 ms 7.4 ms -0.1 ms 0.8%
GPU 1% worst 19.7 ms 19.2 ms 0.5 ms -2.4%
GPU 0.1% worst 25.7 ms 24.2 ms 1.5 ms -6.2% 🔴

@m3taphysics

Copy link
Copy Markdown
Collaborator

PR #8871, run #26639795963

Builds: Windows change, Windows baseline, macOS change, macOS baseline

Framework 13 i7

Metric Change Baseline Delta Improvement
Samples 2699 2700
CPU average 33.3 ms 33.3 ms 0.0 ms -0.0%
CPU 1% worst 34.8 ms 33.8 ms 1.0 ms -3.0%
CPU 0.1% worst 43.8 ms 36.0 ms 7.8 ms -21.6% 🔴
GPU average 7.4 ms 7.4 ms -0.0 ms 0.3%
GPU 1% worst 21.7 ms 20.8 ms 0.9 ms -4.5% 🔴
GPU 0.1% worst 22.9 ms 25.1 ms -2.2 ms 8.7% 🟢

NickKhalow and others added 3 commits June 3, 2026 18:58
Signed-off-by: Nick Khalow <71646502+NickKhalow@users.noreply.github.com>
@m3taphysics

Copy link
Copy Markdown
Collaborator

PR #8871, run #26904756168

Builds: Windows change, Windows baseline, macOS change, macOS baseline

How to read this table
  • Each build is measured 3 times. The values are the median, and (min–max) is the lowest and highest of those runs — a wide range means the metric is noisy and small differences are not trustworthy.
  • Δ is Change minus Baseline (a negative Δ means Change is faster).
  • 🟢 faster / 🔴 slower — a real difference: larger than both 3% and the run-to-run range.
  • ⚪ within noise — the difference is smaller than how much the build varies between its own runs, so it cannot be told apart from random variation. Treat it as no change.
  • Exceptions logged — exceptions found in the run logs; more than the baseline is flagged 🔴 even when frame times look fine.

Framework 13 i7

Metric Baseline Change Δ Result
Samples 2701 (×3) 2701 (×3)
CPU average 33.3 ms 33.3 ms -0.0 ms ⚪ within noise
CPU 1% worst 34.1 ms (33.8–34.1) 33.6 ms (33.5–34.0) -0.5 ms ⚪ within noise
CPU 0.1% worst 39.4 ms (36.3–39.9) 35.2 ms (33.8–36.9) -4.2 ms 🟢 11% faster
GPU average 7.5 ms (7.4–7.8) 8.1 ms (8.1–8.2) 0.6 ms 🔴 8% slower
GPU 1% worst 18.7 ms (18.6–19.6) 17.5 ms (17.4–18.3) -1.2 ms 🟢 6% faster
GPU 0.1% worst 23.3 ms (22.9–24.6) 23.8 ms (22.7–26.5) 0.5 ms ⚪ within noise
Exceptions logged 46 42 -4 🟢 fewer errors

@NickKhalow NickKhalow removed the clean-build Used to trigger clean build on PR label Jun 3, 2026
@m3taphysics

Copy link
Copy Markdown
Collaborator

PR #8871, run #26919679962

Builds: Windows change, Windows baseline, macOS change, macOS baseline

How to read this table
  • Each build is measured 3 times. The values are the median, and (min–max) is the lowest and highest of those runs — a wide range means the metric is noisy and small differences are not trustworthy.
  • Δ is Change minus Baseline (a negative Δ means Change is faster).
  • 🟢 faster / 🔴 slower — a real difference: larger than both 3% and the run-to-run range.
  • ⚪ within noise — the difference is smaller than how much the build varies between its own runs, so it cannot be told apart from random variation. Treat it as no change.
  • Exceptions logged — exceptions found in the run logs; more than the baseline is flagged 🔴 even when frame times look fine.

Framework 13 i7

Metric Baseline Change Δ Result
Samples 2701 (×3) 2701 (×3)
CPU average 33.3 ms 33.3 ms -0.0 ms ⚪ within noise
CPU 1% worst 34.1 ms (34.0–34.5) 33.6 ms (33.5–33.7) -0.5 ms ⚪ within noise
CPU 0.1% worst 39.3 ms (38.4–39.5) 35.0 ms (34.0–36.6) -4.3 ms 🟢 11% faster
GPU average 7.3 ms (7.3–7.5) 8.4 ms (8.2–8.4) 1.1 ms 🔴 15% slower
GPU 1% worst 19.6 ms (19.4–20.1) 17.8 ms (17.3–17.9) -1.9 ms 🟢 10% faster
GPU 0.1% worst 23.3 ms (22.6–25.2) 24.0 ms (20.7–24.6) 0.7 ms ⚪ within noise
Exceptions logged 42 996 +954 🔴 more errors

@NickKhalow NickKhalow changed the base branch from main to dev June 4, 2026 10:11
@NickKhalow NickKhalow changed the title chore: delay on exit fix fix: delay on exit Jun 4, 2026
Comment thread Explorer/Assets/DCL/Infrastructure/Utility/ExitUtils.cs
Co-authored-by: claude[bot] <209825114+claude[bot]@users.noreply.github.com>
Signed-off-by: Nick Khalow <71646502+NickKhalow@users.noreply.github.com>
@claude

This comment has been minimized.

@claude

claude Bot commented Jun 4, 2026

Copy link
Copy Markdown
Contributor

Claude finished @NickKhalow's task in 47s —— View job


Re: CloseHandle on the success path (ExitUtils.cs:247–251)

You're right — the suggested change isn't worth making here, and I'll concede the point.

if (TerminateProcess(handle, 0) == false)
{
    ReportHub.LogProductionInfo($"[ExitUtils] TerminateProcess failed (err {Marshal.GetLastWin32Error()})");
    CloseHandle(handle);
}

The handle targets the current process. When TerminateProcess(handle, 0) succeeds it tears down this process's entire address space and handle table, so:

  • There is no meaningful "leak" — the OS reclaims the handle as part of process teardown.
  • An unconditional CloseHandle(handle) placed after the call would be effectively dead code on the success path, since the process is gone before it could run.
  • The only path where releasing the handle actually matters is the failure path (process survives), and that's already handled.

So the current code is correct as written. My earlier comment already flagged this as "not observable in production" and non-blocking — your pushback is the right call, and no change is needed.

The other two items from the review still stand on their own merits if you want them:

  • macOS errno — already addressed in the current HEAD (Marshal.GetLastPInvokeError() at line 255). ✅
  • +954 exceptions in the latest perf run (41eded1) — worth a fresh perf run against current HEAD to confirm it's resolved, since the DestroyImmediate code that likely caused it has since been removed.
    · branch chore/delay-on-exit-fix

Comment thread Explorer/Assets/DCL/Infrastructure/Utility/ExitUtils.cs
@claude

This comment has been minimized.

@claude

This comment has been minimized.

Comment thread Explorer/Assets/DCL/Infrastructure/Utility/ExitUtils.cs
@github-actions

github-actions Bot commented Jun 4, 2026

Copy link
Copy Markdown
Contributor

🔍 Claude reviewed this PR and found no blocking issues, but assessed it as complex — human DEV review is still required before merging.

@m3taphysics

Copy link
Copy Markdown
Collaborator

PR #8871, run #26951852071

Builds: Windows change, Windows baseline, macOS change, macOS baseline

How to read this table
  • Each build is measured 3 times. The values are the median, and (min–max) is the lowest and highest of those runs — a wide range means the metric is noisy and small differences are not trustworthy.
  • Δ is Change minus Baseline (a negative Δ means Change is faster).
  • 🟢 faster / 🔴 slower — a real difference: larger than both 3% and the run-to-run range.
  • ⚪ within noise — the difference is smaller than how much the build varies between its own runs, so it cannot be told apart from random variation. Treat it as no change.
  • Exceptions logged — exceptions found in the run logs; more than the baseline is flagged 🔴 even when frame times look fine.

Framework 13 i7

Metric Baseline Change Δ Result
Samples 2701 (×3) 2701 (×3)
CPU average 33.3 ms 33.3 ms -0.0 ms ⚪ within noise
CPU 1% worst 33.7 ms (33.6–33.9) 33.6 ms (33.5–33.8) -0.1 ms ⚪ within noise
CPU 0.1% worst 35.8 ms (34.6–36.8) 36.4 ms (35.0–37.1) 0.6 ms ⚪ within noise
GPU average 8.2 ms (7.1–8.5) 8.5 ms (8.4–8.7) 0.3 ms ⚪ within noise
GPU 1% worst 18.5 ms (17.4–20.8) 17.6 ms (17.3–19.0) -0.9 ms ⚪ within noise
GPU 0.1% worst 24.4 ms (21.3–24.6) 21.3 ms (20.9–26.4) -3.0 ms ⚪ within noise
Exceptions logged 46 44 -2 🟢 fewer errors

@claude

This comment has been minimized.

@claude

claude Bot commented Jun 4, 2026

Copy link
Copy Markdown
Contributor

Claude finished @NickKhalow's task in 36s —— View job


Re: Marshal.GetLastPInvokeError()CS0117 ✅ you're right, my suggestion was wrong

You're correct, and I'll retract the earlier suggestion. Marshal.GetLastPInvokeError() was only added in .NET 6. Unity's scripting runtime (Mono / IL2CPP, .NET Standard 2.1 API surface) doesn't expose it, so it fails to compile with exactly the error you posted:

error CS0117: 'Marshal' does not contain a definition for 'GetLastPInvokeError'

The important part: Marshal.GetLastWin32Error() is actually the cross-platform-correct call here — keep it as-is. Despite the "Win32" name, on Mono/IL2CPP the marshaling layer captures the last P/Invoke error from any [DllImport(..., SetLastError = true)] call, and on macOS/Linux that captured value is errno. So ExitUtils.cs:255 already logs the real kill() errno on macOS — no change needed.

In short:

  • GetLastPInvokeError() → doesn't exist in this Unity runtime (your CS0117). ❌
  • GetLastWin32Error() → correct on both Windows and macOS here; reads the errno set by kill(). ✅

The revert to GetLastWin32Error() was the right move. My apologies for the noisy back-and-forth on this — the current code is correct and there's nothing to fix on line 255.
· branch chore/delay-on-exit-fix

@claude

claude Bot commented Jun 4, 2026

Copy link
Copy Markdown
Contributor

PR Review — re-run on latest commits

  • Read CLAUDE.md and docs/README.md
  • Read changed files at current HEAD
  • Analyze exit flow correctness
  • Check for blocking issues
  • Post final summary

View job run

@claude

claude Bot commented Jun 4, 2026

Copy link
Copy Markdown
Contributor

Claude Code is working…

I'll analyze this and get back to you.

View job run

@claude

claude Bot commented Jun 4, 2026

Copy link
Copy Markdown
Contributor

PR Review — re-run on latest commits

  • Read CLAUDE.md and docs/README.md
  • Read changed files at current HEAD
  • Analyze exit flow correctness
  • Check for blocking issues
  • Post final summary

View job run

@claude

claude Bot commented Jun 4, 2026

Copy link
Copy Markdown
Contributor

PR Review — re-run on latest commits

  • Read CLAUDE.md and docs/README.md
  • Read changed files at current HEAD
  • Review new commits since last review (9a28f16d01f39078)
  • Check exit-flow correctness & blocking issues
  • Post final summary

View job run

@claude

claude Bot commented Jun 4, 2026

Copy link
Copy Markdown
Contributor

Claude finished @NickKhalow's task in 2m 56s —— View job


PR Review — re-run on latest commits (a9a63607)

  • Read CLAUDE.md and docs/README.md
  • Read changed files at current HEAD
  • Review new commits since last review (9a28f16da9a63607)
  • Check exit-flow correctness & blocking issues
  • Post final summary

Root-cause check

This PR solves a genuine root cause: Unity's native teardown hangs for 8–11s in Genesis Plaza (confirmed by profiling). The fix bypasses that teardown entirely with TerminateProcess/kill(SIGKILL) after a synchronous graceful cleanup phase. The new commits add diagnostic instrumentation and intentionally skip expensive managed cleanup (avatar TransformAccessArray disposal) on exit, relying on the OS to reclaim native memory. This correctly addresses the actual cause rather than masking it.


Status of all previously raised items

Item Status
CloseHandle on success path Resolved — process is dead before any handle cleanup could run; OS reclaims it.
macOS errno (Marshal.GetLastWin32Error()) Resolved — with [DllImport(..., SetLastError = true)], the Mono/IL2CPP marshaler captures POSIX errno into the same TLS slot that GetLastWin32Error() reads. GetLastPInvokeError() does not exist in this Unity runtime (CS0117). Current code is correct.
+954 exceptions perf regression ResolvedDestroyImmediate path removed; latest run shows 44 vs 46 baseline (fewer).
GPU regression (earlier runs) Noise — most recent run shows all metrics within noise.

New commits reviewed (cd25a0c5a9a63607)

cd25a0c5 — shutdown stopwatch

ShutdownStopwatch struct added. Per-step timing logged via ReportHub.LogProductionInfo. Used as a local variable in MainSceneLoader.Shutdown(), AvatarPlugin.Dispose(), MultiplayerPlugin.Dispose(), AvatarTransformMatrixJobWrapper.Dispose(), and RemoteAvatarPipeline.Dispose(). The struct is mutable (writes stepStartedAtMs in LogStep) — the doc comment warns about copying it. Usage throughout this PR is all local variables, so this is safe in practice.

f66a2aad + df6989ba — drop dispose on about to exit

AvatarTransformMatrixJobWrapper.Dispose() and RemoteAvatarPipeline.Dispose() now early-return when ExitUtils.IsAboutToQuit is true, skipping TransformAccessArray disposal (which is expensive due to managed/native sync). Since TryTerminateSelf fires immediately after Application.Quit(), the OS reclaims all native memory before any managed finalizer could run. This is correct.

The disposed flag is not set on the early-exit path, but since the process is killed immediately after, this is harmless.

01f39078 — restore Complete call

remoteAvatars.Complete() is preserved on the non-exit path in AvatarTransformMatrixJobWrapper.Dispose(). Without this, disposing with pending scheduled jobs would cause a native crash. The early-exit path intentionally skips it (and the disposal entirely), which is acceptable given the hard kill follows immediately.

a9a63607 — disable IsAboutToQuit in editor

IsAboutToQuit returns false unconditionally in UNITY_EDITOR. This is necessary: without this guard, Play Mode exit would trigger the early-exit skip in the avatar Dispose() paths, leaking native resources that accumulate in the Editor process until restart. The doc comment explains this clearly.


No blocking issues found

All previously raised items are closed. The new commits are correct and intentional. The exit flow, cleanup ordering, P/Invoke calls, and Editor safety guard are all sound.


REVIEW_RESULT: PASS ✅
COMPLEXITY: COMPLEX
COMPLEXITY_REASON: Modifies async/managed application exit flow, P/Invoke calls, reflection-based delegate patching, avatar rendering pipeline disposal paths, cross-system cleanup ordering, and ships a vendored native binary.
QA_REQUIRED: YES
· branch chore/delay-on-exit-fix

@claude

claude Bot commented Jun 4, 2026

Copy link
Copy Markdown
Contributor

Claude finished @NickKhalow's task in 2m 45s —— View job


PR Review — new commits since last review (9a28f16da9a63607)

  • Read CLAUDE.md and docs/README.md
  • Reviewed new commits (cd25a0c5 stopwatch, df6989ba / f66a2aad / 01f39078 dispose-on-exit churn, a9a63607 editor guard)
  • Checked exit-flow correctness & blocking issues
  • Dependency / workflow scan for this delta

This round adds the ShutdownStopwatch diagnostic helper, per-step timing logs across the dispose path, and a new leak-on-exit optimization gated by ExitUtils.IsAboutToQuit in the two avatar disposers.


Exit-flow correctness — the new leak guard is sound in the production path ✅

I traced the call graph to confirm the optimization actually fires where intended:

  • MainSceneLoader.Shutdown() is registered as an OnQuittingCleanUpCandidate, so it runs synchronously inside Exit() (after isExiting.Set(true)), before Application.Quit() and the final TryTerminateSelf hard-kill.
  • That means AvatarPlugin.Dispose() → avatarTransformMatrixJobWrapper.Dispose() executes while IsAboutToQuit == true, so the slow TransformAccessArray/NativeArray teardown is correctly skipped, then the process is force-killed and the OS reclaims the leaked native memory. The canShutdown guard also makes the later OnDestroy → Shutdown() a no-op, so there's no double-dispose. Good.

🟡 Non-blocking — leak guard ignores soft-shutdown / non-hard-kill platforms

IsAboutToQuit => isExiting.Value() returns true on every exit in a player build, but the hard TerminateProcess / kill(SIGKILL) only happens on Windows/macOS when useSoftShutdown == false (ExitUtils.cs:242, :236).

In --soft-shutdown mode (and on any other standalone target, e.g. Linux), the disposers still leak TransformAccessArray/NativeArray and skip remoteAvatars.Complete(), but Unity then proceeds with its normal native teardown — which will surface "A Native Collection has not been disposed" leak errors and potential job-safety asserts for the in-flight job. Since the entire justification for leaking is "the process is about to be force-killed," it's cleaner to gate on that actually being true:

// ExitUtils.cs
public static bool IsAboutToQuit => isExiting.Value() && !useSoftShutdown;

Low severity — --soft-shutdown is a diagnostic-only escape hatch, and useSoftShutdown is already file-static in ExitUtils, so the change is trivial and keeps the soft path genuinely "clean" for comparison runs.

🟡 Non-blocking — per-step LogProductionInfo is unconditional on every shutdown

ShutdownStopwatch.LogStep plus the per-plugin / per-avatar-step calls emit a sizable burst of production-info log lines on every exit in release builds. That's exactly what you want while hunting the current root cause, but once Unity support closes out the native-teardown investigation, consider gating the verbose step logging behind the existing diagnostic flag (e.g. useNativeShutdownStopwatch) so it isn't permanent log volume. Keep the single [MainSceneLoader] OnDestroy successfully finished in {ms}ms summary unconditional.

Nits (optional)

  • Typo resoucesresources in both disposer comments (AvatarTransformMatrixJobWrapper.cs:92, RemoteAvatarPipeline.cs:228).
  • AvatarTransformMatrixJobWrapper.cs adds using DCL.Utility; but still writes the fully-qualified DCL.Utility.ExitUtils.IsAboutToQuit; RemoteAvatarPipeline.cs fully-qualifies without adding the using. Pick one for consistency.
  • Trailing whitespace on if (bonesTransformAccessArray.isCreated) (RemoteAvatarPipeline.cs).

Previously-raised items — final status

Item Status
CloseHandle on success path ✅ Resolved by discussion — handle targets the current process; success tears it down before any unconditional close could run.
macOS kill() errno via GetLastWin32Error() ✅ Correct as-is — with SetLastError = true the marshaler captures errno into the same TLS slot this reads; GetLastPInvokeError() doesn't exist in this Unity runtime (CS0117).
+954 exceptions perf regression ✅ Gone — caused by the removed DestroyImmediate path; latest run shows 44 vs 46.

Dependency / workflow scan (this delta)

Only ShutdownStopwatch.cs (+ meta) added — no new packages, binaries, .asmdef, or .github/workflows / prompt changes in this delta. The vendored dcl_exit_stopwatch.exe was assessed in prior rounds (benign, but still shipped without a build-from-source CI step — unchanged LOW–MEDIUM note).

DEPENDENCY_REVIEW: NEEDS_ATTENTION — carried-over native-binary provenance note only; no new HIGH findings.


Verdict

No blocking issues. The leak-on-exit optimization is correct for the production hard-kill path; the only substantive suggestion is to AND the guard with !useSoftShutdown so the soft/diagnostic path stays clean. Everything else is nits.

REVIEW_RESULT: PASS ✅
· branch chore/delay-on-exit-fix

@m3taphysics

This comment has been minimized.

@m3taphysics

Copy link
Copy Markdown
Collaborator

PR #8871, run #26958730327

Builds: Windows change, Windows baseline, macOS change, macOS baseline

How to read this table
  • Each build is measured 3 times. The values are the median, and (min–max) is the lowest and highest of those runs — a wide range means the metric is noisy and small differences are not trustworthy.
  • Δ is Change minus Baseline (a negative Δ means Change is faster).
  • 🟢 faster / 🔴 slower — a real difference: larger than both 3% and the run-to-run range.
  • ⚪ within noise — the difference is smaller than how much the build varies between its own runs, so it cannot be told apart from random variation. Treat it as no change.
  • Exceptions logged — exceptions found in the run logs; more than the baseline is flagged 🔴 even when frame times look fine.

Framework 13 i7

Metric Baseline Change Δ Result
Samples 2701 (×3) 2701 (×3)
CPU average 33.3 ms 33.3 ms -0.0 ms ⚪ within noise
CPU 1% worst 34.1 ms (33.6–34.3) 33.5 ms (33.4–33.6) -0.5 ms ⚪ within noise
CPU 0.1% worst 39.3 ms (35.5–41.7) 34.7 ms (33.6–36.2) -4.6 ms ⚪ within noise
GPU average 7.6 ms (7.4–8.2) 8.7 ms (8.2–9.0) 1.0 ms 🔴 14% slower
GPU 1% worst 19.7 ms (18.4–20.2) 17.5 ms (16.7–18.7) -2.2 ms 🟢 11% faster
GPU 0.1% worst 23.5 ms (23.0–25.5) 20.9 ms (18.7–24.9) -2.6 ms ⚪ within noise
Exceptions logged 50 44 -6 🟢 fewer errors

@DafGreco DafGreco left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

✔️ PR reviewed and approved by QA on both platforms following instructions playing both happy and un-happy path

Regressions for this ticket had been performed in order to verify that the normal flow is working as expected:

  • [ ✔️] Backpack and wearables in world
  • [ ✔️] Emotes in world and in backpack
  • [✔️ ] Teleport with map/coordinates/Jump In
  • [ ✔️] Chat and multiplayer
  • [ ✔️] Profile card
  • [ ✔️] Camera
  • [ ✔️] Skybox
  • [✔️ ] Settings

Stopwatch only appears on windows not in macos . More information in the following thread

Evidence:

Image Image

@NickKhalow NickKhalow merged commit bc87c12 into dev Jun 5, 2026
18 of 22 checks passed
@NickKhalow NickKhalow deleted the chore/delay-on-exit-fix branch June 5, 2026 07:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

force-build Used to trigger a build on draft PR new-dependency

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants