fix: make lifecycle calls on a destroyed task safe no-ops#600
Merged
Conversation
Calling start() right after destroy() re-armed the runner's heartbeat before the state machine rejected the transition, so the timer kept firing forever and every slot logged an invalid-transition error. The task is already out of the registry at that point, so shutdown() can't reach it either. start() now returns early on a destroyed task instead of touching the runner.
destroy() followed by stop() rejected with "invalid transition from destroyed to stopped": stop() emits task:stopped even without a fork process, and the constructor's handler for that event unconditionally tried to move the state machine out of destroyed. start() had the same gap and would fork a fresh daemon for a task that was already torn down. Both now resolve immediately without touching the fork, and the task:stopped handler no longer attempts a transition once destroyed.
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
1 participant
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.
Summary
Two related bugs where calling a lifecycle method after
destroy()did the wrong thing instead of being a safe no-op:task.destroy(); task.start();re-armed the runner's heartbeat before the state machine rejected the transition. The timer never died, and every scheduled slot logged an "invalid transition from destroyed to running" error forever. Since the task is already removed from the registry at that point,cron.shutdown()cannot reach it either.await task.destroy(); await task.stop();rejected with "invalid transition from destroyed to stopped", becausestop()emitstask:stoppedeven without a fork process, and the constructor's handler for that event unconditionally tried to move the state machine out ofdestroyed. Sincecron.shutdown()callsstop()on every registered task, a manualdestroy()followed byshutdown()could hit this. Backgroundstart()afterdestroy()also re-forked a fresh daemon for an already torn-down task.Both
start()andstop()are now safe no-ops on a destroyed task (inline and background), matching the inlinestop()behavior that was already correct.destroy()itself was already idempotent, so no change was needed there.