dotnet / dotnet/msbuild

BuildEventContext has no build discriminator, so a logger reused across builds produces a corrupt log

Open
#14,609 2 comments 0 reactions 0 assignees View on GitHub
backlog triaged
Dominant language
C#
Stars
5.5k
Forks
1.5k
Avg merge
1d 8h
Merged PRs (30d)
141

Description

Hosts that drive MSBuild through the object model commonly attach a single logger across several builds. `ProjectInstance.Build(targets, loggers)` is a full `BeginBuild`/request/`EndBuild` per call, so a host that invokes a target on N projects performs N separate builds while holding one logger.

The resulting log is corrupt, and there is no way for a reader to recover from it.

## Why the ids collide

Each `BeginBuild` creates a new `LoggingService`, and its id counters are *instance* fields seeded to fixed values:

```csharp
// src/Build/BackEnd/Components/Logging/LoggingService.cs
_nextProjectId = nodeId;
_nextEvaluationId = nodeId;

private int _nextTargetId = 1;
private int _nextTaskId = 1;
```

So `ProjectContextId`, `TargetId` and `TaskId` all restart on every build. A single `.binlog` written by one logger across N builds contains N `BuildStarted`/`BuildFinished` pairs whose contexts collide. Readers key project and target nodes on those ids, so every project's targets get attributed to whichever project claimed the id first, and the remaining projects appear to have executed nothing.

Observed on a two-project run:

```
BuildStarted
ProjectStarted (sub=0, pctx=4) T2.csproj TargetIds 2,3,4,5
BuildFinished
BuildStarted
ProjectStarted (sub=1, pctx=4) T1.csproj TargetIds 2,3,4,5 <-- identical
BuildFinished
```

## There is no discriminator to key on

`BuildEventContext` exposes `NodeId`, `TargetId`, `ProjectContextId`, `TaskId`, `ProjectInstanceId`, `SubmissionId` and `EvaluationId`. There is no build or session id.

`SubmissionId` does differ across the builds above, but it is explicitly excluded from identity:

```csharp
// src/Framework/BuildEventContext.cs
public override int GetHashCode()
{
var hash = 17;
// submission ID does not contribute to equality
// hash = hash * 31 + _submissionId;
hash = (hash * 31) + _nodeId;
...
```

`InternalEquals` drops it as well, so two contexts originating from different builds compare **equal**. It is not simply that `BuildEventContext` lacks a `BuildId` — the one field that varies is deliberately removed from identity, leaving nothing usable. `public long BuildRequestId => GetHashCode();` inherits the same blind spot despite its name.

Related: the `ILogger` contract compounds this, since `Initialize`/`Shutdown` are invoked per build. dotnet/sdk works around that half with a `FacadeLogger`/`PersistentDispatcher` pair whose comment states it plainly: *"the MSBuild API for evaluation and execution calls logger Initialize and Shutdown methods, so will not allow us to do this."* The lifecycle can be worked around from outside the engine; the ids cannot.

## Real-world impact

dotnet/sdk#55187 — `dotnet test -bl` produced binlogs in which every test project's targets collapsed onto a single project. Worked around in dotnet/sdk#55521 by routing all requests through one `BuildManager` session so only one build is ever emitted, but that is a per-call-site fix; `dotnet run` has the same latent issue, as does any host reusing a logger across builds.

## Possible directions

1. Add a build/session discriminator to `BuildEventContext` and include it in `Equals`/`GetHashCode`, so a logger can safely span builds and `Target 1 in build A` is distinguishable from `Target 1 in build B`.
2. Make id allocation monotonic across builds within a single `BuildManager`.
3. If reusing a logger across builds is unsupported by contract, detect it and fail fast when a logger is initialized a second time, and document the restriction.

Happy to help with whichever direction you prefer.

/cc @baronfel — this follows from https://github.com/dotnet/sdk/pull/55521#issuecomment-5120126234

Contributor guide

No contributing guide indexed for this repository

Research direction

Start with src/Build/BackEnd/Components/Logging/LoggingService.cs and src/Framework/BuildEventContext.cs, then trace the BeginBuild/request/EndBuild lifecycle used by ProjectInstance.Build(targets, loggers). Reproduce the two-project, reused-logger scenario described in the issue and compare context identity across builds. Done means an agreed design prevents contexts from different builds colliding, with corresponding regression coverage.

Written by the indexing model from the issue text.

Assessment

Tech stack
csharp
Domain
build-system
Issue type
Bug
Difficulty
5/5
Estimated time
Over a week
Activity status
Quiet
Clarity
Needs clarification
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.