Skip to content

Fix CF vitest async leaks behind the intermittent gate hang; restore CodeQL on PRs - #5671

Merged
norman-abramovitz merged 2 commits into
cloudfoundry:developfrom
nabramovitz:fix/cf-vitest-async-leaks
Jul 22, 2026
Merged

Fix CF vitest async leaks behind the intermittent gate hang; restore CodeQL on PRs#5671
norman-abramovitz merged 2 commits into
cloudfoundry:developfrom
nabramovitz:fix/cf-vitest-async-leaks

Conversation

@nabramovitz

Copy link
Copy Markdown
Contributor

The cloud-foundry vitest project intermittently hung a fork worker for ~17 minutes, blowing the PR gate from ~20 to 34+ minutes. Running the project without --dangerouslyIgnoreUnhandledErrors surfaced 21 unhandled errors that the gate flag normally swallows; 20 of them shared one root cause.

Stepper first-step poller leak (20/21 errors). The steppers component starts a 5ms setInterval waiting for the first step to unblock, held only in a local variable. It was never cleared on destroy: if the step never unblocked it polled forever, and a tick after teardown called afterNextRender against a destroyed injector (NG0205: Injector has already been destroyed). Every wizard-hosting spec (application-delete, detach-service-instance, add-service-instance, edit-space, edit-organization, add-quota, add-organization) surfaced it. The same pattern would throw in production on a fast navigate-away from any wizard. The timer is now a field, cleared in ngOnDestroy, with a destroyed-check in the tick. The interval polling created enough stray async under the shared 3-worker fork pool to intermittently wedge a worker — the spec that happened to be mid-beforeEach then ate the 15s hook timeout and the worker never recovered.

Cell metric null deref (1/21). cloud-foundry-cell-base mapped metric.bosh_job_id without a null guard while the adjacent isLoading$ pipe already defends the same stream. Now metric?.bosh_job_id ?? ''.

Spec teardown. cf-apps-signal-config.service.spec reset TestBed only in beforeEach, so the last test's module (with live stats timer/effects) survived until worker shutdown. Added the afterEach reset; the service's existing DestroyRef cleanup does the rest.

Verified with three consecutive full cloud-foundry runs: 1745 passed, exit 0, zero unhandled errors (previously 21 errors, exit 1).

CI changes

  • codeql.yml: restore the push + pull_request triggers. CodeQL was removed from PRs on the theory it was involved in the gate slowness; the investigation above exonerated it (its job consistently finished in ~3 minutes), so PRs get scanned again. The workflow_dispatch trigger is kept for on-demand runs.
  • pr.yml: add a per-ref concurrency group with cancel-in-progress, so a push while a gate is running cancels the stale run instead of contending with it for runners.

…c-leak fixes

The steppers component's first-step unblock setInterval was a local that
survived component destruction: if the step never unblocked it polled
forever, and a post-destroy tick called afterNextRender against a destroyed
injector (NG0205). Under the shared 3-worker vitest fork pool those leaks
destabilized workers and intermittently hung the cloud-foundry project ~17
minutes (the >34min PR gate). 20 of the 21 unhandled errors in a full CF
run traced here through 8 wizard-hosting specs.

Also: null-guard the cell metric name mapping (the 21st unhandled error),
and destroy the cf-apps-signal-config spec's last TestBed module so its
stats timer/effects don't outlive the test file.

Verified: 3 consecutive full cloud-foundry runs, 1745 passed, exit 0,
zero unhandled errors (previously 21 + exit 1).

@norman-abramovitz norman-abramovitz 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.

LGTM

@norman-abramovitz
norman-abramovitz merged commit 2593608 into cloudfoundry:develop Jul 22, 2026
22 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants