fix: prevent batch unlinking twice#18303
Merged
Merged
Conversation
Merged #18298 a bit too soon - there's still a situation which can corrput the linked list: If we merge one batch into another, it will unlink the batch immediately. But `discard` also calls it, running the unlink logic again, which can corrupt the linked list. Sketch: 1. Batch A is pending. 2. Batch B starts later, intersects A, resolves first, and merges into A. 3. B is unlinked immediately, but `A.oncommit(() => B.discard())` remains. 4. Independent batch C is created while A is still pending and is linked after A. 5. A commits, calls `B.discard()`, and B's stale `#prev`/`#next` can set `A.#next = null` / `last_batch = A`, disconnecting C. Not able to produce a failing test from it but it's definitely a fix we need to make.
|
Contributor
|
Rich-Harris
approved these changes
May 27, 2026
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
2 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.
Merged #18298 a bit too soon - there's still a situation which can corrput the linked list: If we merge one batch into another, it will unlink the batch immediately. But
discardalso calls it, running the unlink logic again, which can corrupt the linked list.Sketch:
A.oncommit(() => B.discard())remains.B.discard(), and B's stale#prev/#nextcan setA.#next = null/last_batch = A, disconnecting C.Not able to produce a failing test from it but it's definitely a fix we need to make.
Also moved
#linkinto a constructor, because it is (and should be) used only once.Also made the action after an error
discardinstead of just#unlinkbecause this batch is done for and e.g. pendingsettledshould resolve, too.