Skip to content

Fix nil pointer dereference in Windows pipe writer Close - #397

Merged
gh-worker-dd-mergequeue-cf854d[bot] merged 3 commits into
masterfrom
mrafi/fix-pipe-windows
Aug 14, 2026
Merged

gh-worker-dd-mergequeue-cf854d[bot] merged 3 commits into
masterfrom
mrafi/fix-pipe-windows

Conversation

@mrafi97

@mrafi97 mrafi97 commented Aug 14, 2026 •

Copy link
Copy Markdown
Collaborator

What does this PR do?

Adds a nil guard to pipeWriter.Close() on Windows.

newWindowsPipeWriter deliberately defers connection setup to the first
write, so conn is nil until then. Closing a client that never wrote
dereferenced that nil connection and panicked. udsWriter.Close()
already has this exact guard; this brings the pipe writer in line.

The connection is now read under p.mu rather than directly, because
Write() nils the field out under a write lock when a non-temporary
error disconnects it — an unsynchronized read in Close() is a data
race.

Motivation

#incident-59243
https://datadoghq.atlassian.net/browse/WINA-3031

Found via a crashing Windows service in datadog-agent. Its pre-flight
mode component dials the named pipe once to probe readiness, then the
statsd client dials again on first write. When that second dial loses a
startup race, the deferred client.Close() took down the process with
0xc0000005 — a read of address 0x18, i.e. a method call through a
nil interface.

Not a recent regression: conn: nil dates to c995f1b (March 2021,
first released in v4.5.0), and pipe_windows.go is byte-identical
between v5.8.3 and v5.9.0. It had simply never had a caller that closes
a client which never successfully wrote.

Testing

Adds TestPipeWriterCloseWithoutWrite, which constructs a writer and
closes it without writing — this panics on the current code. Verified
go vet and a test binary build under GOOS=windows; the pipe tests
themselves need a real Windows named pipe, so they only execute on a
Windows runner.

Guard against a nil connection in pipeWriter.Close(). The writer defers
connection setup to the first write, so conn is nil until then, and
closing a client that never wrote panicked with a nil pointer
dereference. This mirrors the guard udsWriter.Close() already has.

Read conn under p.mu rather than directly: Write() nils the field out
under a write lock when a non-temporary error disconnects it, so an
unsynchronized read in Close() is a data race.

Signed-off-by: Mohammad Rafi <mohammad.rafi@datadoghq.com>
@datadog-prod-us1-6

datadog-prod-us1-6 Bot commented Aug 14, 2026 •

Copy link
Copy Markdown

Code Coverage

🎯 Code Coverage (details)
• Patch Coverage: 100.00%
• Overall Coverage: 84.95% (+0.25%)

This comment will be updated automatically if new data arrives.
🔗 Commit SHA: c3f1f6e | Docs | Datadog PR Page | Give us feedback!

@mrafi97
mrafi97 marked this pull request as ready for review August 14, 2026 16:35
@mrafi97
mrafi97 requested a review from a team as a code owner August 14, 2026 16:35
rayz
rayz previously approved these changes Aug 14, 2026
Comment thread statsd/pipe_windows.go Outdated
Co-authored-by: Jesse Szwedko <jesse@szwedko.me>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@gh-worker-dd-mergequeue-cf854d
gh-worker-dd-mergequeue-cf854d Bot merged commit 3873e5d into master Aug 14, 2026
99 checks passed
@gh-worker-dd-mergequeue-cf854d
gh-worker-dd-mergequeue-cf854d Bot deleted the mrafi/fix-pipe-windows branch August 14, 2026 21:06
gh-worker-dd-mergequeue-cf854d Bot pushed a commit to DataDog/datadog-agent that referenced this pull request Aug 17, 2026
### What does this PR do?

- Upgrades `github.com/DataDog/datadog-go/v5` from `v5.9.0` to `v5.9.1` across the Go workspace.

### Motivation

[#incident-59243](https://dd.enterprise.slack.com/archives/C0BQAB1T274)
https://datadoghq.atlassian.net/browse/WINA-3031

`datadog-go v5.9.1` fixes a nil pointer dereference in the Windows named-pipe
writer (DataDog/datadog-go#397). The pipe connection is established on first
write, so closing a client that never successfully wrote dereferenced a nil
`net.Conn` and terminated the process.

The Agent Data Plane pre-flight probe triggers this on Windows. It dials the
pipe once to prove readiness, then the statsd client dials again on its first
write; when that second dial loses a startup race, the deferred
`client.Close()` crashed the Agent service with `0xc0000005`. This is
intermittent by nature — it needs the first dial to succeed and the second to
fail.

### Describe how you validated your changes

- `go list -m github.com/DataDog/datadog-go/v5` resolves to `v5.9.1`.
- `go mod verify` — all modules verified.
- `GOOS=windows go build ./comp/dataplane/...` passes. The fix is in a
  `//go:build windows` file, so a native build would not exercise it.
- Confirmed every changed `go.mod`/`go.sum` line is datadog-go and nothing else.

### Additional Notes

Renovate normally owns this bump (it did v5.9.0 in #53234), but
`minimumReleaseAge: 7 days` means it would not open a PR until ~2026-08-24.
Landing it manually since the crash blocks 7.84. The diff matches Renovate's
output file-for-file, so it will simply see the dependency as current.


Co-authored-by: mohammad.rafi <mohammad.rafi@datadoghq.com>
github-actions Bot pushed a commit to DataDog/datadog-agent that referenced this pull request Aug 17, 2026
### What does this PR do?

- Upgrades `github.com/DataDog/datadog-go/v5` from `v5.9.0` to `v5.9.1` across the Go workspace.

### Motivation

[#incident-59243](https://dd.enterprise.slack.com/archives/C0BQAB1T274)
https://datadoghq.atlassian.net/browse/WINA-3031

`datadog-go v5.9.1` fixes a nil pointer dereference in the Windows named-pipe
writer (DataDog/datadog-go#397). The pipe connection is established on first
write, so closing a client that never successfully wrote dereferenced a nil
`net.Conn` and terminated the process.

The Agent Data Plane pre-flight probe triggers this on Windows. It dials the
pipe once to prove readiness, then the statsd client dials again on its first
write; when that second dial loses a startup race, the deferred
`client.Close()` crashed the Agent service with `0xc0000005`. This is
intermittent by nature — it needs the first dial to succeed and the second to
fail.

### Describe how you validated your changes

- `go list -m github.com/DataDog/datadog-go/v5` resolves to `v5.9.1`.
- `go mod verify` — all modules verified.
- `GOOS=windows go build ./comp/dataplane/...` passes. The fix is in a
  `//go:build windows` file, so a native build would not exercise it.
- Confirmed every changed `go.mod`/`go.sum` line is datadog-go and nothing else.

### Additional Notes

Renovate normally owns this bump (it did v5.9.0 in #53234), but
`minimumReleaseAge: 7 days` means it would not open a PR until ~2026-08-24.
Landing it manually since the crash blocks 7.84. The diff matches Renovate's
output file-for-file, so it will simply see the dependency as current.

Co-authored-by: mohammad.rafi <mohammad.rafi@datadoghq.com> 4718b57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants