Avoid destroying in-memory alarm tasks while running - #7215
Conversation
|
All contributors have signed the CLA ✍️ ✅ |
|
I have read the CLA Document and I hereby sign the CLA |
JosephDoUrden
left a comment
There was a problem hiding this comment.
Pulled a4d2c98, in-memory test passes, same abort on cd90354 without the fix, alarm and name tests green.
updateAlarmInMemory has two branches and this only fixes the none one. worker.c++:4261 still assigns retry() straight into maybeAlarmPreviewTask. Call setAlarm inside alarm() on inMemory and it lands there, same abort at kj/async.c++:2187.
Second thing, the evalLater result is stored and never awaited, so it never runs. Deleted alarm still fires its timer, actor-cache.c++:183 alarm handler canceled shows on your branch, not base.
@xThreeh can you cover the reschedule branch too, and hand the old task to a task set that runs it, like #6934 did.
Fixes the reported case, not mergeable until the reschedule branch is covered.
When an in-memory Durable Object alarm completes, cleanup can clear the preview task while its callback is still running. That destroys the active promise callback and aborts workerd with Promise callback destroyed itself. Defer the task cleanup to the next event-loop turn instead. Added a regression test covering an empty alarm handler with in-memory storage. Tests: bazel test //src/workerd/api/tests:actor-alarms-in-memory-test@; bazel test //src/workerd/api/tests:actor-alarms-test@; python tools/cross/format.py --check git. Fixes #7190