Skip to content

Exit DrainPacketQueue thread when build completes.#12561

Merged
JanProvaznik merged 2 commits intodotnet:mainfrom
Erarndt:dev/erarndt/closeDrainPacketQueueThread
Nov 4, 2025
Merged

Exit DrainPacketQueue thread when build completes.#12561
JanProvaznik merged 2 commits intodotnet:mainfrom
Erarndt:dev/erarndt/closeDrainPacketQueueThread

Conversation

@Erarndt
Copy link
Contributor

@Erarndt Erarndt commented Sep 23, 2025

Fixes #

Context

Currently, the dedicated thread that handles processing the packet queue doesn't exit properly when build finishes. The threads are idle, but repeated builds create a new thread each time and they accumulate during the session:

image

The check for IsExitPacket() is slightly misleading since it checks for more than just the completion of the build

        private static bool IsExitPacket(INodePacket packet)
        {
            return packet is NodeBuildComplete buildCompletePacket && !buildCompletePacket.PrepareForReuse;
        }

By explicitly checking for the NodeBuildComplete packet, we can exit the processing loop and let the thread get cleaned up. Additionally, I named the dedicated thread to help identify issues like this in the future.

Changes Made

Testing

Notes

Copilot AI review requested due to automatic review settings September 23, 2025 17:31
Copy link
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

This PR fixes a thread management issue where DrainPacketQueue threads were not being properly terminated when builds complete, causing thread accumulation during repeated builds. The fix ensures the dedicated thread exits when it receives a NodeBuildComplete packet.

Key changes:

  • Added explicit thread termination logic for NodeBuildComplete packets
  • Added thread naming for better debugging visibility

@rainersigwald
Copy link
Member

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@JanProvaznik JanProvaznik merged commit b86f357 into dotnet:main Nov 4, 2025
10 checks passed
@Erarndt Erarndt deleted the dev/erarndt/closeDrainPacketQueueThread branch November 4, 2025 17:32
@rainersigwald
Copy link
Member

/backport to vs18.3

@github-actions
Copy link
Contributor

github-actions bot commented Nov 4, 2025

Started backporting to vs18.3 (link to workflow run)

SimaTian added a commit that referenced this pull request Nov 6, 2025
Backport of #12561 to vs18.3

/cc @rainersigwald @Erarndt

---------

Co-authored-by: Eric Arndt <erarndt@microsoft.com>
Co-authored-by: Tomas Bartonek <tom.bartonek@volny.cz>
@rainersigwald
Copy link
Member

/backport to vs18.0

@github-actions
Copy link
Contributor

Started backporting to vs18.0 (link to workflow run)

rainersigwald added a commit that referenced this pull request Nov 14, 2025
Backport of #12561 to vs18.0

Work item (Internal use):
[devdiv#2613541](https://devdiv.visualstudio.com/DevDiv/_workitems/edit/2613541)

### Summary

MSBuild communications threads were incorrectly not exiting when builds
completed.

### Customer Impact

Memory leak that gets worse with each build or design-time build
operation.

### Regression?

Yes, from 17.14.

### Testing

Unit tests, manual testing, already inserted into 18.3 builds.

### Risk

Low.

---------

Co-authored-by: Eric Arndt <erarndt@microsoft.com>
Co-authored-by: Rainer Sigwald <raines@microsoft.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.

4 participants