Skip to content

sessions: Avoid resolving canceled Changes editor inputs - #333647

Merged
Benjamin Christopher Simmonds (benibenj) merged 1 commit into
mainfrom
agents/log-analysis-error-fix-prioritization-cf89cb99
Sep 1, 2026
Merged

sessions: Avoid resolving canceled Changes editor inputs#333647
Benjamin Christopher Simmonds (benibenj) merged 1 commit into
mainfrom
agents/log-analysis-error-fix-prioritization-cf89cb99

Conversation

@benibenj

Copy link
Copy Markdown
Contributor

Summary

  • stop the Sessions Changes editor from resolving an input after its open operation has been canceled
  • add a regression test that verifies canceled inputs never request their multi-diff view model

Root cause

A newer editor operation can cancel an in-flight SessionChangesEditor.setInput while it is yielding in the base setInput call. Cleanup can then clear and dispose the old SessionChangesEditorInput. When the canceled continuation resumed, it attempted to lazily create an inner multi-diff input through an already disposed MutableDisposable, then crashed while reading getViewModel from the rejected value.

Validation

  • npm run transpile-client
  • .\scripts\test.bat --run src\vs\sessions\contrib\changes\test\browser\sessionChangesEditorInput.test.ts (5 passing)

A canceled setInput operation can outlive editor cleanup and try to resolve an already disposed SessionChangesEditorInput. Stop before model resolution and cover the race with a regression test.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI balanced review requested due to automatic review settings August 31, 2026 21:32

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

Review tier: Balanced
Findings: None

What changed in this PR

Prevents canceled Sessions Changes editor operations from resolving disposed inputs.

Changes:

  • Adds a cancellation check after the base input setup.
  • Adds regression coverage ensuring canceled inputs do not request view models.
File Description
sessionChangesEditor.ts Stops processing canceled input operations.
sessionChangesEditorInput.test.ts Tests canceled-input behavior.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@dmitrivMS

Copy link
Copy Markdown
Collaborator

Is this related?
#333541

@benibenj

Copy link
Copy Markdown
Contributor Author

Yes—related, but complementary rather than duplicates.

#333541 stops SinglePaneDockedTabsCoordinator work that remains queued/in flight after the coordinator or editor group is disposed. Its reported failure occurs before/during pane creation (InstantiationService has been disposed).

This PR covers a later, editor-pane-owned cancellation boundary: SessionChangesEditor.setInput has already started and yields in super.setInput; a newer editor operation cancels it, clears/disposes that input, and then the old continuation resumes and calls getViewModel. The logs show this repeatedly during ordinary session switches without the InstantiationService error. Also, #333541 can only check isCancelled() after its awaited replaceEditors/openEditor returns, so it cannot stop the canceled setInput continuation inside that await.

The regression test here exercises that cancellation directly and is independent of the coordinator. So #333541 prevents one producer of stale editor work, while this check makes the Changes editor correctly honor the editor framework's cancellation token for that and other producers.

@benibenj
Benjamin Christopher Simmonds (benibenj) merged commit 3e33b21 into main Sep 1, 2026
38 checks passed
@benibenj
Benjamin Christopher Simmonds (benibenj) deleted the agents/log-analysis-error-fix-prioritization-cf89cb99 branch September 1, 2026 07:22
@vs-code-engineering vs-code-engineering Bot added this to the 1.137.0 milestone Sep 1, 2026
Bhavya U (bhavyaus) pushed a commit that referenced this pull request Sep 1, 2026
A canceled setInput operation can outlive editor cleanup and try to resolve an already disposed SessionChangesEditorInput. Stop before model resolution and cover the race with a regression test.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants