make test sometimes fails with a data race on time.Local in cli.Entry #192

Closed
opened 2026-10-15 17:07:34 +02:00 by clawbot · 3 comments
Collaborator

Since #169 (commit 42f4961), cli.Entry() sets time.Local = time.UTC every time it runs (internal/cli/root.go:20). The tests call Entry() in the same process as other tests, and a timer goroutine left by an earlier test can be calling time.Now(), which reads time.Local, at that moment. The race detector then fails TestEntryWipesBuffersAndReturnsExitCode (internal/cli/entry_test.go:40), and with it make test. It happens on some runs of an unchanged next, not all: the write is at root.go:20 and the read is in time.Now() called from time.sendTime().

Definition of done

  • The time zone is made UTC where no other goroutine can be running, or every time secret prints or writes is put in UTC without changing time.Local, and the race detector reports nothing in internal/cli across repeated make test runs.

Model: opus-5-5

Since https://git.eeqj.de/sneak/secret/issues/169 (commit `42f4961`), `cli.Entry()` sets `time.Local = time.UTC` every time it runs (`internal/cli/root.go:20`). The tests call `Entry()` in the same process as other tests, and a timer goroutine left by an earlier test can be calling `time.Now()`, which reads `time.Local`, at that moment. The race detector then fails `TestEntryWipesBuffersAndReturnsExitCode` (`internal/cli/entry_test.go:40`), and with it `make test`. It happens on some runs of an unchanged `next`, not all: the write is at `root.go:20` and the read is in `time.Now()` called from `time.sendTime()`. ## Definition of done - The time zone is made UTC where no other goroutine can be running, or every time `secret` prints or writes is put in UTC without changing `time.Local`, and the race detector reports nothing in `internal/cli` across repeated `make test` runs. Model: opus-5-5
Author
Collaborator

The same spot also fails a second way: on another run, the internal/cli test binary died with signal: segmentation fault right after the output of the test before TestEntryWipesBuffersAndReturnsExitCode. Entry() ends with memguard.Purge(), which destroys every memguard buffer in the process, so a buffer still in use by a goroutine left from an earlier test in the same binary would fault. Unverified that this is the cause; it is the likeliest reading. Whatever fixes this issue should cover both.

Model: opus-5-5

The same spot also fails a second way: on another run, the `internal/cli` test binary died with `signal: segmentation fault` right after the output of the test before `TestEntryWipesBuffersAndReturnsExitCode`. `Entry()` ends with `memguard.Purge()`, which destroys every memguard buffer in the process, so a buffer still in use by a goroutine left from an earlier test in the same binary would fault. Unverified that this is the cause; it is the likeliest reading. Whatever fixes this issue should cover both. Model: opus-5-5
Author
Collaborator

Plan. One PR on next; it comes before other work, since make test on next fails on some runs.

  • time.Local = time.UTC moves out of cli.Entry() into main() in cmd/secret, before cli.Entry() runs, where no other goroutine exists yet. The test from #169 runs the built binary with TZ=America/New_York, so it still covers every printed time being UTC. Entry()'s doc and the nolint comment move with the line.
  • TestEntryWipesBuffersAndReturnsExitCode runs Entry() in a process of its own (the test binary running itself, the usual Go way), so memguard.Purge() cannot destroy a buffer another test in the same binary still holds. The test still checks both exit codes and that a buffer made before Entry() is wiped.
  • No other behaviour changes. One Completed Steps entry at the top of TODO.md.

Done when: the race detector reports nothing in internal/cli, and neither the race nor the crash appears, across five script/cibuild runs in a row.

Model: opus-5-5

**Plan.** One PR on `next`; it comes before other work, since `make test` on `next` fails on some runs. - `time.Local = time.UTC` moves out of `cli.Entry()` into `main()` in `cmd/secret`, before `cli.Entry()` runs, where no other goroutine exists yet. The test from https://git.eeqj.de/sneak/secret/issues/169 runs the built binary with `TZ=America/New_York`, so it still covers every printed time being UTC. `Entry()`'s doc and the `nolint` comment move with the line. - `TestEntryWipesBuffersAndReturnsExitCode` runs `Entry()` in a process of its own (the test binary running itself, the usual Go way), so `memguard.Purge()` cannot destroy a buffer another test in the same binary still holds. The test still checks both exit codes and that a buffer made before `Entry()` is wiped. - No other behaviour changes. One Completed Steps entry at the top of `TODO.md`. Done when: the race detector reports nothing in `internal/cli`, and neither the race nor the crash appears, across five `script/cibuild` runs in a row. Model: opus-5-5
Author
Collaborator

Built as planned in #194. One detail the plan did not settle: the copy of the test binary that runs cli.Entry() acts in an init function, not in the test, so that TestMain does not build the secret binary again and remove it in that copy while the other tests run it.

Model: opus-5-5

Built as planned in https://git.eeqj.de/sneak/secret/pulls/194. One detail the plan did not settle: the copy of the test binary that runs `cli.Entry()` acts in an `init` function, not in the test, so that `TestMain` does not build the `secret` binary again and remove it in that copy while the other tests run it. Model: opus-5-5
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: sneak/secret#192