containers: Honor subclass sleepAfter override during initial activity setup#192
Merged
gabivlj merged 1 commit intocloudflare:mainfrom Apr 28, 2026
Merged
Conversation
…y setup Subclass class-field initializers (e.g. `sleepAfter = "2h"`) run after the base class constructor returns. The base constructor calls `renewActivityTimeout()` synchronously inside its `blockConcurrencyWhile` callback, so `this.sleepAfter` is always the base default (`"10m"`) when the initial `sleepAfterMs` is computed. Subclass overrides take effect on the next `renewActivityTimeout` call (e.g. from the first `containerFetch`), but if the first alarm fires before any fetch, the container can be killed early — the repro in cloudflare#127. Swap the order: `await scheduleNextAlarm()` first — the await yields a microtask, which lets super() return and subclass class-field initializers run — then `renewActivityTimeout()` reads the now-overridden `this.sleepAfter`. `scheduleNextAlarm` doesn't depend on `sleepAfterMs`, so the reorder is safe. Fixes cloudflare#127.
quantizor
force-pushed
the
fix/sleepafter-subclass-init-race
branch
from
0a83a51 to
cf41295
Compare
Collaborator
|
Hey! - Appreicate the PR! We will review this shortly and hopefully get it merged/release |
commit: |
gabivlj
approved these changes
Apr 28, 2026
Contributor
Author
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #127.
Root cause
Subclass class-field initializers (
sleepAfter = '2h') run after the base class constructor returns. The base constructor callsrenewActivityTimeout()synchronously inside itsblockConcurrencyWhilecallback, at a point wherethis.sleepAfteris still the base default ('10m'). The initialsleepAfterMsis therefore always computed from the default, not the subclass override.Reported timeline in #127 (
sleepAfter = '2h', container sleeps after 1–2 minutes) is consistent with this: the first alarm fires inside the 10-minute base-default window before anycontainerFetchhas had a chance to callrenewActivityTimeout()with the correct value.Fix
Swap two statements in the constructor's
blockConcurrencyWhilecallback soscheduleNextAlarm()runs beforerenewActivityTimeout():The
awaityields a microtask, during whichsuper()returns and subclass class-field initializers run. By the timerenewActivityTimeout()executes,this.sleepAfteris the subclass-overridden value.scheduleNextAlarmdoesn't depend onsleepAfterMs, so the reorder is safe.Tests
Verified locally against a red/green cycle with a targeted Jest test that constructs a subclass with
sleepAfter = '2h', awaits the constructor'sblockConcurrencyWhilepromise, and assertssleepAfterMsis ~2h out. Not included here because the Jest harness in this repo currently lives alongside #191's test infrastructure — once either this PR or #191 lands, the regression can be pinned in a follow-up. Happy to include the test if you'd prefer the harness land with this PR.