fix: dispose never-shown AuthenticationScreenController without NRE

[alejandro-jimenez-dcl]

Problem

At app exit, MainSceneLoader.Shutdown logs "MainUIPlugin's thrown an exception on
disposal" with an inner NullReferenceException from
AuthenticationScreenController.Dispose (Sentry UNITY-EXPLORER-PC4, 44 events / 8 users
past week). Worse than the log line: the first throwing controller aborts
MVCManager.Dispose's loop, so every controller registered after it (~200 total) leaks its
disposal - event unsubscribes, CTS cancels - on every affected exit.

Root cause

AuthenticationScreenController.Dispose unconditionally disposes fsm and audio, which
are only assigned in OnViewInstantiated() - and view instantiation is lazy and optional.
Sessions started with --skip-auth-screen plus a valid cached identity (the Creator Hub
preview flow) never show the auth screen, so both fields are still null at shutdown. The
field annotations lied about the lifecycle: null-until-shown is the real invariant.

Fix (~21 LOC, 2 files)

1. AuthenticationScreenController: declare fsm/audio nullable with the
   null-until-shown invariant; Dispose() uses audio?.Dispose(); fsm?.Dispose();
   (mirroring the adjacent characterPreviewController?.Dispose()); post-instantiation
   lifecycle sites use !. per the trust-nullability convention (ControllerBase guarantees
   show-lifecycle callbacks run only after OnViewInstantiated). An annotation correction,
   not a defensive null-check.
2. MVCManager.Dispose (defect-class hardening): per-controller try/catch +
   ReportHub.LogException(..., ReportCategory.MVC) so one throwing controller can no
   longer truncate disposal of the remaining controllers, the destruction CTS, and the
   windows stack.

Test

• AuthenticationScreenControllerShould.NotThrowOnDisposeWhenViewWasNeverShown - constructs
  the controller with a never-invoked view factory and disposes; RED at pin (NRE from
  audio.Dispose()).
• MVCManagerDisposeShould.DisposeRemainingControllersAndStackWhenOneControllerThrows -
  order-independent: a throwing controller must not prevent the other controller and the
  windows stack from being disposed.

Validation

Windows Unity 6000.4 EditMode lane at the pin: RED FAIL 2/2 as intended (NRE at
AuthenticationScreenController.Dispose; the substitute's exception aborting
MVCManager.Dispose) / GREEN PASS 2/2.

Related: #9030 (stale-closed tracker for this exact Sentry issue, still occurring), #8972
(same defect, earlier symbolication), #6942 (closed umbrella for the shutdown-disposal
class)

[decentraland-bot]

PR Review — #9802 fix: dispose never-shown AuthenticationScreenController without NRE

STEP 2 — Root-cause check: ✅ PASS

The PR correctly identifies and fixes the root cause: fsm and audio are only assigned in OnViewInstantiated(), which is lazy and optional (sessions using --skip-auth-screen with a valid cached identity never trigger it). The field declarations lied about their lifecycle — they were declared non-nullable but were null in a legitimate runtime scenario. The fix corrects the annotations to match the real invariant (null-until-shown), which is a proper cause fix, not a symptom patch.

The MVCManager.Dispose hardening addresses a secondary defect in the same failure chain: a single throwing controller aborting disposal of all subsequent controllers (~200 total). The per-controller try/catch is a defect-class fix, not a workaround.

STEP 3 — Design & integration: ✅ PASS

No new long-lived units are introduced. Both changes modify existing classes:

1. AuthenticationScreenController — A nullability annotation correction on two private fields. No new state, no new lifecycle, no new subscriptions.
2. MVCManager.Dispose — A standard resilience pattern (per-item try/catch in a disposal loop). Logged via ReportHub.LogException(e, ReportCategory.MVC) per project conventions.

Teardown/consumption trace:

• audio?.Dispose() / fsm?.Dispose() — these are the teardown for objects created in OnViewInstantiated. When the view was never instantiated, both are null and the ?. correctly skips them. When instantiated, both are disposed. ✅
• No new subscriptions, events, connections, or buffers are introduced.

STEP 4 — Member audit: ✅ PASS

No new public members are added. The four events (DiscordButtonClicked, OTPVerified, OTPResend, ProfileFinalized) are existing declarations whose annotations are corrected from Action to Action? — C# events are null by default when no subscribers are attached, so this is a truthful annotation. RaiseProfileFinalized() already used ?.Invoke() before this PR.

STEP 5 — Line-level review: ✅ No issues found

Null-forgiving operators (!): The PR adds 8 ! usages on fsm and audio across lifecycle methods (OnBeforeViewShow, OnViewShow, OnViewClose, TryAutoLoginAndProceedAsync, ChangeAccountAsync). These are justified:

• Invariant documented at the field declaration: // Null until OnViewInstantiated: the view is created lazily on first Show and may never be instantiated.
• Lifecycle guarantee verified in ControllerBase.LaunchViewLifeCycleAsync(): OnViewInstantiated() always runs before OnBeforeViewShow() and OnViewShow(); HideViewAsync() (which calls OnViewClose()) can only execute after LaunchViewLifeCycleAsync was called. TryAutoLoginAndProceedAsync and ChangeAccountAsync are only reachable from OnBeforeViewShow and the view's UI respectively — both post-instantiation.
• This follows the trust-nullability convention per CLAUDE.md: the annotation is correct (T?), the ! is used only where the lifecycle guarantees non-null, and the invariant is documented. This is an annotation correction, not a defensive null-check.

MVCManager.Dispose try/catch: Catches Exception broadly, which is correct for a disposal loop — you want to ensure cleanup continues regardless of what a single controller throws. The exception is logged (not swallowed) via ReportHub.LogException with the correct ReportCategory.MVC.

Disposal order: audio?.Dispose() before fsm?.Dispose() is consistent — audio is created first in OnViewInstantiated, and mirrors the adjacent characterPreviewController?.Dispose() pattern.

Tests:

• AuthenticationScreenControllerShould.NotThrowOnDisposeWhenViewWasNeverShown — Minimal, focused. Uses null! for all constructor params because the constructor only stores dependencies (none are dereferenced before view instantiation). Directly tests the RED scenario described in the PR.
• MVCManagerDisposeShould.DisposeRemainingControllersAndStackWhenOneControllerThrows — Clean NSubstitute setup, proper LogAssert.ignoreFailingMessages for the expected ReportHub error log, verifies both the surviving controller and the windows stack are disposed. Uses existing namespace-level test helpers (ITestView, TestInputData from MVCManagerShould.cs) and defines IOtherTestView for the second controller registration — consistent with existing patterns.

STEP 3 (security pass): ✅ No security issues found

The changes are in disposal paths and nullable annotations. No secrets, no user input handling, no auth/authz logic changes, no sensitive data exposure.

STEP 6 — Complexity

COMPLEX — The diff touches MVCManager.Dispose, which is a resource cleanup path.

STEP 7 — QA assessment

YES — Both files are runtime code that ships in the Unity player. The auth screen disposal and MVC manager disposal execute during app shutdown.

---

REVIEW_RESULT: PASS ✅
COMPLEXITY: COMPLEX
COMPLEXITY_REASON: Modifies MVCManager.Dispose (resource cleanup path) and AuthenticationScreenController disposal lifecycle.
QA_REQUIRED: YES

---

Reviewed by Jarvis 🤖 · Requested by decentraland-bot via GitHub

[github-actions]

🚦 CI Status

Build skipped — no changes detected under Explorer/.

No C# files changed — lint ratchet skipped.

All Unity tests passed ✅

TESTS SUITE	Result	Passed	Failed	Skipped
EditMode	✅ Passed	25056	0	13
PlayMode	✅ Passed	236	0	37

[github-actions]

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

[decentraland-bot]

PR #9802, run #32266856354

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 per run — the average number of exceptions in a run's log; more than the baseline is flagged 🔴 even when frame times look fine. The Exception breakdown under each table groups them by the explorer's report category and exception type (as totals across the runs).
• A run that logged unusually many exceptions (at least 10 and 5× the median of its build's runs — e.g. a service was down during it) is excluded from all numbers and called out under the table.

Intel Core i5

Metric	Baseline	Change	Δ	Result
Samples	2313 (×3)	1615 (×3)
CPU average	38.6 ms (33.4–38.8)	54.6 ms (41.9–55.0)	16.0 ms	🔴 41% slower
CPU 1% worst	322.6 ms (57.0–343.5)	978.7 ms (894.1–984.9)	656.1 ms	🔴 203% slower
CPU 0.1% worst	344.1 ms (341.1–360.3)	1037.2 ms (917.4–1054.3)	693.1 ms	🔴 201% slower
GPU average	9.5 ms (9.2–9.6)	10.8 ms (10.2–10.8)	1.2 ms	🔴 13% slower
GPU 1% worst	35.6 ms (23.5–37.7)	96.4 ms (72.0–96.7)	60.9 ms	🔴 171% slower
GPU 0.1% worst	44.4 ms (39.8–45.0)	99.0 ms (96.4–99.0)	54.6 ms	🔴 123% slower
Exceptions per run	66	65	-1	🟢 fewer errors

Exception breakdown

Exception	Baseline (3 runs)	Change (3 runs)
[UI] DllNotFoundException	192	192
[ENGINE] ObjectDisposedException	3	3
[ENGINE] NullReferenceException	3	0

Apple M1

Metric	Baseline	Change	Δ	Result
Samples	4105 (×3)	4112 (×3)
CPU average	21.8 ms (21.8–22.9)	21.8 ms (21.3–21.9)	-0.1 ms	⚪ within noise
CPU 1% worst	215.9 ms (215.7–217.7)	214.4 ms (100.6–216.7)	-1.5 ms	⚪ within noise
CPU 0.1% worst	226.3 ms (222.9–228.8)	236.2 ms (234.4–237.5)	9.9 ms	🔴 4% slower
GPU average	2.5 ms (2.0–3.2)	2.4 ms (1.2–3.1)	-0.1 ms	⚪ within noise
GPU 1% worst	34.3 ms (34.2–36.2)	34.7 ms (34.3–35.5)	0.4 ms	⚪ within noise
GPU 0.1% worst	36.3 ms (34.8–37.5)	36.7 ms (35.6–37.8)	0.4 ms	⚪ within noise
Exceptions per run	0	0	0	⚪ none new

[alejandro-jimenez-dcl]

Approved