Skip to content

fix: resolve data race on the process exit code#139

Merged
joshiste merged 2 commits into
mainfrom
fix/processstate-data-race
Jun 30, 2026
Merged

fix: resolve data race on the process exit code#139
joshiste merged 2 commits into
mainfrom
fix/processstate-data-race

Conversation

@joshiste

Copy link
Copy Markdown
Member

Problem

Start reaped the process in a goroutine (go cmd.Wait(), which writes cmd.ProcessState) while Status/Stop read cmdState.Cmd.ProcessState.ExitCode() concurrently — a data race that go test -race flags and that can yield torn reads of the process result.

Fix

Use the shared extcmd.CmdState.Wait() / ExitCode() API added in extension-kit#149: CmdState owns the process result, Wait() is the sole reader of cmd.ProcessState, and ExitCode() returns it race-free (-1 while running). Start now does go cmdState.Wait(); the handlers read cmdState.ExitCode().

This is the same fix applied across the four extensions that share this extcmd pattern (extension-gatling, extension-jmeter, extension-k6, extension-postman).

Dependency

extension-kit is pinned to the unreleased commit (v1.10.6-0.20260630192502-059086860afd) until #149 is merged and tagged; the pin should be bumped to the tagged release before merge.

Verification

go build ./..., go vet ./..., go test -race ./... pass.

Start reaped the process in a goroutine (go cmd.Wait(), which writes
cmd.ProcessState) while Status/Stop read cmdState.Cmd.ProcessState
concurrently — a data race flagged by go test -race. Use the shared
extcmd.CmdState.Wait()/ExitCode() API: the Wait goroutine is the sole
reader of ProcessState and the handlers read the exit code race-free.
The liveness Signal(0) check still reads cmd.Process, which Wait does
not write, so it is unaffected.
@claude

claude Bot commented Jun 30, 2026

Copy link
Copy Markdown

Claude finished @joshiste's task in 1m 39s —— View job


Claude's Review

  • Check for previously unresolved review threads (none found)
  • Review diff for correctness, security, performance, and concurrency issues
  • Post review feedback

Summary: This is a clean, minimal fix for the data race described in the PR body. I traced the change through extpostman/action_run.go:

  • Start (line 252): the reaper goroutine now calls cmdState.Wait() instead of cmd.Wait(), making CmdState the sole owner/writer of the process result.
  • Status (line 274) and Stop (line 368): both now read the exit code via cmdState.ExitCode() instead of directly touching cmdState.Cmd.ProcessState.ExitCode(), removing the concurrent read of a field written by the goroutine.

This correctly eliminates the race between the reaping goroutine and the status/stop handlers, consistent with the new extcmd.CmdState API contract (Wait() is the only reader of cmd.ProcessState; ExitCode() is race-free and returns -1 while running). I didn't find any other site in this file still touching cmd.Wait() or ProcessState directly, so the fix is complete for this extension.

No bugs, security issues, or new concurrency problems found in the diff. Two notes, neither blocking:

  • go.mod/go.sum pin extension-kit to an unreleased pseudo-version (v1.10.6-0.20260630192502-059086860afd) — the PR description already flags this is intentional and temporary until extension-kit#149 is tagged, so just a reminder to bump it before merging.
  • The pre-existing result.Completed = false unconditional overwrite in Status (line 288, just below the still-running branch) looks like a latent logic quirk, but it predates this PR and is out of scope here.

I was unable to run go build/go vet/go test -race myself (tool permissions in this sandbox don't allow invoking go), so please rely on CI to confirm the verification steps mentioned in the PR description.

@sonarqubecloud

Copy link
Copy Markdown

@joshiste joshiste merged commit aaeac64 into main Jun 30, 2026
@github-actions github-actions Bot locked and limited conversation to collaborators Jun 30, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant