monitorTarget / performHealthCheck mutate a Target's Status, LastError, LastCheck, CheckCount, and consecutive-success/failure counters from the target's monitor goroutine. Meanwhile getAllTargetsUnsafe (called by GetTargets, used for status reporting to the server) copied the whole struct with targetCopy := *target.
The monitor mutex protects the targetsmap, not the contents of each Target, so these accesses overlap without synchronization. The race detector flags it on any concurrent monitor + GetTargets workload — e.g. the reconnect path, where the status reporter snapshots targets while their monitor goroutines are updating status.
WARNING: DATA RACE
Read at ... by goroutine N:
healthcheck.(*Monitor).getAllTargetsUnsafe() // targetCopy := *target
Previous write at ... by goroutine M:
healthcheck.(*Monitor).monitorTarget() // target.timer / target.Status
Fix
Add a per-Targetsync.Mutex guarding the mutable status fields.
Replace the whole-struct copy with a snapshot() taken under that lock, copying only the reported fields (Config, Status, LastCheck, LastError, CheckCount). Runtime-only fields (timer/ctx/cancel/client) are intentionally omitted — callers of GetTargets only read the reported fields, and this also avoids copying a sync.Mutex.
The lock is not held across the health-check network request, so status snapshots are never blocked for the check timeout.
DisableTarget's Status write is guarded too.
Test
Adds healthcheck_test.go with a regression test that exercises concurrent monitoring + snapshotting. It fails under -race without the fix and passes with it:
# without the fix
--- FAIL: TestConcurrentStatusSnapshotNoRace (race detected)
# with the fix
ok github.com/fosrl/newt/healthcheck
go build, go vet ./healthcheck/, and gofmt are clean.
No functional/behavioral change to health checking — this is purely a concurrency-safety fix.
🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.
## 📋 Pull Request Information
**Original PR:** https://github.com/fosrl/newt/pull/412
**Author:** [@lafoush](https://github.com/lafoush)
**Created:** 7/14/2026
**Status:** 🔄 Open
**Base:** `main` ← **Head:** `fix/healthcheck-status-data-race`
---
### 📝 Commits (1)
- [`a4017be`](https://github.com/fosrl/newt/commit/a4017be6431c6a5db5500f6e33c8c1d366236681) fix(healthcheck): guard target status fields against data race
### 📊 Changes
**2 files changed** (+117 additions, -4 deletions)
<details>
<summary>View changed files</summary>
📝 `healthcheck/healthcheck.go` (+34 -4)
➕ `healthcheck/healthcheck_test.go` (+83 -0)
</details>
### 📄 Description
## Problem
`monitorTarget` / `performHealthCheck` mutate a `Target`'s `Status`, `LastError`, `LastCheck`, `CheckCount`, and consecutive-success/failure counters from the target's monitor goroutine. Meanwhile `getAllTargetsUnsafe` (called by `GetTargets`, used for status reporting to the server) copied the whole struct with `targetCopy := *target`.
The monitor mutex protects the `targets` **map**, not the contents of each `Target`, so these accesses overlap without synchronization. The race detector flags it on any concurrent monitor + `GetTargets` workload — e.g. the reconnect path, where the status reporter snapshots targets while their monitor goroutines are updating status.
```
WARNING: DATA RACE
Read at ... by goroutine N:
healthcheck.(*Monitor).getAllTargetsUnsafe() // targetCopy := *target
Previous write at ... by goroutine M:
healthcheck.(*Monitor).monitorTarget() // target.timer / target.Status
```
## Fix
- Add a per-`Target` `sync.Mutex` guarding the mutable status fields.
- Replace the whole-struct copy with a `snapshot()` taken under that lock, copying only the reported fields (`Config`, `Status`, `LastCheck`, `LastError`, `CheckCount`). Runtime-only fields (`timer`/`ctx`/`cancel`/`client`) are intentionally omitted — callers of `GetTargets` only read the reported fields, and this also avoids copying a `sync.Mutex`.
- The lock is **not** held across the health-check network request, so status snapshots are never blocked for the check timeout.
- `DisableTarget`'s `Status` write is guarded too.
## Test
Adds `healthcheck_test.go` with a regression test that exercises concurrent monitoring + snapshotting. It **fails under `-race` without the fix and passes with it**:
```
# without the fix
--- FAIL: TestConcurrentStatusSnapshotNoRace (race detected)
# with the fix
ok github.com/fosrl/newt/healthcheck
```
`go build`, `go vet ./healthcheck/`, and `gofmt` are clean.
No functional/behavioral change to health checking — this is purely a concurrency-safety fix.
---
<sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
📋 Pull Request Information
Original PR: https://github.com/fosrl/newt/pull/412
Author: @lafoush
Created: 7/14/2026
Status: 🔄 Open
Base:
main← Head:fix/healthcheck-status-data-race📝 Commits (1)
a4017befix(healthcheck): guard target status fields against data race📊 Changes
2 files changed (+117 additions, -4 deletions)
View changed files
📝
healthcheck/healthcheck.go(+34 -4)➕
healthcheck/healthcheck_test.go(+83 -0)📄 Description
Problem
monitorTarget/performHealthCheckmutate aTarget'sStatus,LastError,LastCheck,CheckCount, and consecutive-success/failure counters from the target's monitor goroutine. MeanwhilegetAllTargetsUnsafe(called byGetTargets, used for status reporting to the server) copied the whole struct withtargetCopy := *target.The monitor mutex protects the
targetsmap, not the contents of eachTarget, so these accesses overlap without synchronization. The race detector flags it on any concurrent monitor +GetTargetsworkload — e.g. the reconnect path, where the status reporter snapshots targets while their monitor goroutines are updating status.Fix
Targetsync.Mutexguarding the mutable status fields.snapshot()taken under that lock, copying only the reported fields (Config,Status,LastCheck,LastError,CheckCount). Runtime-only fields (timer/ctx/cancel/client) are intentionally omitted — callers ofGetTargetsonly read the reported fields, and this also avoids copying async.Mutex.DisableTarget'sStatuswrite is guarded too.Test
Adds
healthcheck_test.gowith a regression test that exercises concurrent monitoring + snapshotting. It fails under-racewithout the fix and passes with it:go build,go vet ./healthcheck/, andgofmtare clean.No functional/behavioral change to health checking — this is purely a concurrency-safety fix.
🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.