Make telemetry initialization and disposal best-effort so it can never fail a build - #14554
Draft
AlesProkop with Copilot wants to merge 2 commits into
Draft
Make telemetry initialization and disposal best-effort so it can never fail a build#14554AlesProkop with Copilot wants to merge 2 commits into
AlesProkop with Copilot wants to merge 2 commits into
Conversation
Contributor
|
Hello @copilot, I noticed that you’re changing an .swr file or any file under src/Package/MSBuild.VSSetup.. Please make sure to validate this change by an experimental VS insertion. This is accomplished by pushing to an exp/* branch, which requires write permissions to this repo. |
Fixes the MSB1025 build failure caused by a NullReferenceException escaping VsTelemetryInitializer.Dispose(), and stops BuildManager.Dispose() from tearing down the process-wide telemetry session it does not own. Co-authored-by: AlesProkop <276576870+AlesProkop@users.noreply.github.com>
Copilot
AI
changed the title
[WIP] Fix unhandled exception in VsTelemetryInitializer.Dispose
Make telemetry initialization and disposal best-effort so it can never fail a build
Jul 28, 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
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.
Context
MSBuild 18.8.2 on .NET Framework fails builds with
MSBUILD : error MSB1025when aNullReferenceExceptionescapesVsTelemetryInitializer.Dispose(). Two independent defects combine here:TelemetryManagerswallowed the wrong exceptions. Both init and dispose used a filtered catch (FileNotFoundException/FileLoadException/TypeLoadException), which doesn't match an NRE thrown from inside the VS telemetry stack. The exception propagated up throughBuildManager.Dispose()toMSBuildApp.Executeand failed the build.BuildManagerdisposed a session it doesn't own. The telemetry session is process-wide, created by the entry point (XMake.Main) or the host (VS).MSBuildApp.BuildProject'sfinallydisposesBuildManager.DefaultBuildManager, so the session was torn down while the process was still running — leavingCrashTelemetryRecorder.PostFaultEventposting to a disposed session, which is the second stack trace in the report.Telemetry is best-effort infrastructure; no failure inside it should be observable by a build.
Changes Made
src/Framework/Telemetry/TelemetryManager.csTryInitializeTelemetryandDisposenow use a catch-all instead of a type-filtered catch. Comments record which exceptions are expected and why the filter isn't sufficient.s_disposedis set before the teardown work, andDefaultActivitySourceis cleared, so nothing can emit through a source backed by a disposed session.VsTelemetryInitializer.Initializeno longer dereferences the session unconditionally.CreateAndGetDefaultSessioncan return null (e.g. telemetry disabled machine-wide), soUseVsIsOptedIn()/Start()and ownership are only taken when a session actually exists:VsTelemetryInitializer.Disposesnapshots and clears the static state before disposing, so a throwingDispose()can't leave a stale session behind and a repeat call is a no-op.using System;/using System.IO;, now unused (IDE0005 is warning-as-error in official builds).src/Build/BackEnd/BuildManager/BuildManager.csTelemetryManager.Instance.Dispose()fromDispose(bool). This is a no-op change for hosts, which initialize withisStandalone: falseand therefore never owned the session;XMake.MainandMSBuild.Coordinator/Program.csalready pair their own Initialize/Dispose, and on .NET CoreDispose()has no body.Testing
src/Framework.UnitTests/TelemetryManager_Tests.cs: disposal without initialization doesn't throw, disposal is idempotent,ResetForTestclears state.BuildManagerDisposeDoesNotDisposeProcessWideTelemetryinsrc/Build.UnitTests/Telemetry/Telemetry_Tests.cs, verified to fail when the removedTelemetryManager.Instance.Dispose()call is restored.Notes
This is defensive. The underlying reason
TelemetrySession.Dispose()NREs in the reporter's environment (Windows Server 2022, VS 2026 + VS 2022 side-by-side,VS_TELEMETRY_OPT_OUT=1) is in the closed-source VS Telemetry SDK and is not addressed here — but no failure mode inside it can fail a build anymore.Worth a look during review:
BuildManager.EndBuildTelemetry()callsStartActivitywithout a guard. Left alone as out of scope, but it's adjacent.