Skip to content

Add logic to keep track of COM callbacks and cancel them if the sessi…#40183

Merged
OneBlue merged 6 commits intofeature/wsl-for-appsfrom
user/oneblue/com-cancel
Apr 16, 2026
Merged

Add logic to keep track of COM callbacks and cancel them if the sessi…#40183
OneBlue merged 6 commits intofeature/wsl-for-appsfrom
user/oneblue/com-cancel

Conversation

@OneBlue
Copy link
Copy Markdown
Collaborator

@OneBlue OneBlue commented Apr 14, 2026

…on is terminating

Summary of the Pull Request

This change adds logic to cancel user COM callbacks when the session terminates. This will prevent a session from being "stuck" if a user callback hangs during termination

PR Checklist

  • Closes: Link to issue #xxx
  • Communication: I've discussed this with core contributors already. If work hasn't been agreed, this work might be rejected
  • Tests: Added/updated if needed and all pass
  • Localization: All end user facing strings can be localized
  • Dev docs: Added/updated if needed
  • Documentation updated: If checked, please file a pull request on our docs repo and link it here: #xxx

Detailed Description of the Pull Request / Additional comments

Validation Steps Performed

Copilot AI review requested due to automatic review settings April 14, 2026 23:49
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

This PR adds tracking of threads that are executing outgoing COM callbacks so that Terminate() can cancel those calls (via CoCancelCall) and avoid session shutdown hangs caused by stuck user callbacks.

Changes:

  • Introduces an RAII helper (UserCOMCallback) to enable COM call cancellation and register/unregister callback threads in WSLCSession.
  • Cancels outstanding outgoing COM calls during WSLCSession::Terminate().
  • Adds a Windows test that simulates a stuck progress callback and validates it is unblocked by session termination.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 8 comments.

File Description
test/windows/WSLCTests.cpp Adds a regression test for stuck COM progress callback cancellation during Terminate().
src/windows/wslcsession/WSLCSession.h Adds UserCOMCallback and session APIs/state for tracking callback threads.
src/windows/wslcsession/WSLCSession.cpp Implements callback registration, enables/disables call cancellation, and cancels calls during termination.

Comment thread src/windows/wslcsession/WSLCSession.cpp
Comment thread src/windows/wslcsession/WSLCSession.cpp
Comment thread src/windows/wslcsession/WSLCSession.cpp
Comment thread src/windows/wslcsession/WSLCSession.cpp Outdated
Comment thread src/windows/wslcsession/WSLCSession.cpp
Comment thread src/windows/wslcsession/WSLCSession.cpp
Comment thread test/windows/WSLCTests.cpp Outdated
Comment thread test/windows/WSLCTests.cpp
Copilot AI review requested due to automatic review settings April 15, 2026 03:44
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.

Comment thread src/windows/wslcsession/WSLCSession.cpp
Comment thread src/windows/wslcsession/WSLCSession.cpp Outdated
Comment thread src/windows/wslcsession/WSLCSession.cpp Outdated
Comment thread test/windows/WSLCTests.cpp
@OneBlue OneBlue marked this pull request as ready for review April 15, 2026 20:51
@OneBlue OneBlue requested a review from a team as a code owner April 15, 2026 20:51
Copilot AI review requested due to automatic review settings April 15, 2026 20:51
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.

Comment on lines +51 to +67
class UserCOMCallback
{
NON_COPYABLE(UserCOMCallback);

public:
UserCOMCallback(WSLCSession& Session) noexcept;
UserCOMCallback(UserCOMCallback&& Other) noexcept;

~UserCOMCallback() noexcept;

UserCOMCallback& operator=(UserCOMCallback&& Other) noexcept;
void Reset() noexcept;

private:
WSLCSession* m_session{};
DWORD m_threadId{};
};
Copy link

Copilot AI Apr 15, 2026

Choose a reason for hiding this comment

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

UserCOMCallback references WSLCSession before WSLCSession is declared in this header. This is a compile error unless there is an earlier forward declaration not shown; add class WSLCSession; before UserCOMCallback (or move UserCOMCallback below the WSLCSession declaration).

Copilot uses AI. Check for mistakes.
Comment thread src/windows/wslcsession/WSLCSession.cpp
Comment thread src/windows/wslcsession/WSLCSession.cpp
Comment thread test/windows/WSLCTests.cpp
Comment thread test/windows/WSLCTests.cpp
Copy link
Copy Markdown
Member

@JohnMcPMS JohnMcPMS left a comment

Choose a reason for hiding this comment

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

This in the CoEnableCallCancellation docs seems concerning:

When call cancellation is enabled on a thread, marshaled synchronous calls from that thread to objects on the same computer can suffer serious performance degradation.

Comment thread src/windows/wslcsession/WSLCSession.cpp
@OneBlue
Copy link
Copy Markdown
Collaborator Author

OneBlue commented Apr 15, 2026

This in the CoEnableCallCancellation docs seems concerning:

When call cancellation is enabled on a thread, marshaled synchronous calls from that thread to objects on the same computer can suffer serious performance degradation.

Agreed, although in our case we're not expecting an enormous amount of calls, this is still better than potentially hanging because a client is stuck

@OneBlue OneBlue merged commit 3168f4c into feature/wsl-for-apps Apr 16, 2026
7 checks passed
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