Skip to content

Keep the scheduler hook session alive until its disposers run - #446

Merged
Maximo-Guk merged 2 commits into
mainfrom
maximo/scheduler-hook-disposal-flake
Sep 4, 2026
Merged

Keep the scheduler hook session alive until its disposers run#446
Maximo-Guk merged 2 commits into
mainfrom
maximo/scheduler-hook-disposal-flake

Conversation

@Maximo-Guk

@Maximo-Guk Maximo-Guk commented Sep 4, 2026

Copy link
Copy Markdown
Member

Fixes the schedule-driver.test.ts disposal-count flake (e.g. https://github.com/cloudflare/cloudflare-os/actions/runs/33825527399/job/100877250818).

workerd aborts the startHook RPC session as "hung" the moment the driver releases the returned capabilities, racing the task that runs their Symbol.dispose, when the abort wins, the disposer never runs. The test initiator now holds the session open until both disposers have run, and the tests await one in-Worker waitForDisposals that reports the counts reached on failure. Test fixture only.

Reproduced 1 in 60 runs under CPU load before; 0 in 100 after.

The two ScheduleDriver tests that count server-side disposals of the hook
capabilities flaked with an opaque 5s timeout. workerd runs an RpcTarget's
disposer as a task of the RPC session that created it and aborts a non-actor
session as "hung" once its last pending event is gone; the returned
capabilities were that session's only pending events, so releasing them armed
the abort, which raced the disposer task and occasionally cancelled it.

The test initiator now holds the session open with a pending timer until both
disposers have run, and the tests await an in-Worker `waitForDisposals` that
reports the counts reached and the hook timeline instead of polling with a
budget equal to the test timeout.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@github-actions github-actions Bot added the gatekeeper Changes to a gatekeeper integration label Sep 4, 2026
ask-bonk[bot]

This comment was marked as resolved.

@ask-bonk

ask-bonk Bot commented Sep 4, 2026

Copy link
Copy Markdown

Posted 1 actionable inline finding.

github run

@github-actions

github-actions Bot commented Sep 4, 2026

Copy link
Copy Markdown

Preview: pr446-maximo-schedu-f620c41c

https://pr446-maximo-schedu-f620c41c-router.cloudflare-os-previews.workers.dev

Dashboard · deleted when this PR closes

`waitForDisposals` now requires the counts to equal the target once reached,
so an extra disposer invocation fails instead of passing, and each hook
capability records the reset generation it was minted under so a disposer
from an earlier test that lands after `reset()` is not counted.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@ask-bonk

ask-bonk Bot commented Sep 4, 2026

Copy link
Copy Markdown

LGTM!

github run

@Maximo-Guk
Maximo-Guk marked this pull request as ready for review September 4, 2026 15:49

@devin-ai-integration devin-ai-integration Bot 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.

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no bugs or issues to report.

Devin Review

@Maximo-Guk
Maximo-Guk merged commit 00da2d6 into main Sep 4, 2026
18 checks passed
@Maximo-Guk
Maximo-Guk deleted the maximo/scheduler-hook-disposal-flake branch September 4, 2026 15:54
modataconsulting added a commit to MO-Data-Consulting/cloudflare-os that referenced this pull request Sep 4, 2026
* Let a test command raise its watchdog idle threshold (cloudflare#442)

* Let a test command raise its watchdog idle threshold

* Keep the scheduler hook session alive until its disposers run (cloudflare#446)

* Keep the scheduler hook session alive until its disposers run

* Fix eval config startup (cloudflare#447)

---------

Co-authored-by: Maximo Guk <62088388+Maximo-Guk@users.noreply.github.com>
Co-authored-by: Ashish Kumar Singh <ashishsingh@cloudflare.com>
modataconsulting added a commit to MO-Data-Consulting/cloudflare-os that referenced this pull request Sep 8, 2026
* Let a test command raise its watchdog idle threshold (cloudflare#442)

* Let a test command raise its watchdog idle threshold

* Keep the scheduler hook session alive until its disposers run (cloudflare#446)

* Keep the scheduler hook session alive until its disposers run

* Fix eval config startup (cloudflare#447)

* chore: bump Workers compatibility date (cloudflare#448)

* Render selected skills as composer pills (cloudflare#422)

* feat(DESENG-0000): render selected skills as composer pills

* fix(DESENG-0000): simplify composer skill pills

* fix(frontend): preserve composer spell checking

* Add composer skill picker (cloudflare#423)

* feat(frontend): add composer skill picker

* fix(frontend): restore composer skill picker behavior

* fix(DESENG-0000): avoid clipped picker focus outline

* fix(frontend): harden composer skill picker

* fix(frontend): refine composer options menu

* test(frontend): send unconfirmed slash text plainly

* fix(frontend): refresh composer skill catalogs

* fix(frontend): preserve plain slash submission

* fix(frontend): activate composer option on Tab

* fix(frontend): refine composer picker layout

* feat(frontend): show skill context in slash picker

* fix(frontend): preserve composer overlay focus

* fix(frontend): avoid stale composer picker state

* Fix Outputs sidebar not stretching to full height (cloudflare#454)

---------

Co-authored-by: Maximo Guk <62088388+Maximo-Guk@users.noreply.github.com>
Co-authored-by: Ashish Kumar Singh <ashishsingh@cloudflare.com>
Co-authored-by: Nathan Disidore <nathan@cloudflare.com>
Co-authored-by: Bálint Ferenczy <40574454+FBalint@users.noreply.github.com>
Co-authored-by: Phillip Jones <pjones@cloudflare.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gatekeeper Changes to a gatekeeper integration

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants