Skip to content

fix: Fix data race on Windows#4051

Open
gmlewis wants to merge 1 commit intogoogle:masterfrom
gmlewis:i4050-go-test-race
Open

fix: Fix data race on Windows#4051
gmlewis wants to merge 1 commit intogoogle:masterfrom
gmlewis:i4050-go-test-race

Conversation

@gmlewis
Copy link
Collaborator

@gmlewis gmlewis commented Feb 27, 2026

This is an AI-assisted fix for the data race detected during the GitHub Actions workflows in #4050.
Since this is not an easy data race to understand (at least for me), I asked it to add comments as to why the changes being made are necessary to prevent the data race both in the CI/CD pipeline and potentially when using the library in production.

cc: @stevehipwell - @alexandear - @zyfy29 - @Not-Dhananjay-Mishra - @munlicode

The original data race is below for posterity:

Details

From #4050 CI/CD:

Run if [ -n "" ]; then
  if [ -n "" ]; then
    script/test.sh -race -covermode atomic -coverprofile coverage.txt ./...
    exit
  fi
  script/test.sh -race -covermode atomic ./...
  shell: C:\Program Files\Git\bin\bash.EXE --noprofile --norc -e -o pipefail {0}
  env:
    GO111MODULE: on
    GOTOOLCHAIN: local
testing .
go: downloading github.com/google/go-querystring v1.2.0
go: downloading github.com/google/go-cmp v0.7.0
==================
WARNING: DATA RACE
Read at 0x00c005271b40 by goroutine 8856:
  internal/poll.(*FD).execIO()
      C:/hostedtoolcache/windows/go/1.26.0/x64/src/internal/poll/fd_windows.go:269 +0x284
  internal/poll.(*FD).Read()
      C:/hostedtoolcache/windows/go/1.26.0/x64/src/internal/poll/fd_windows.go:586 +0x353
  net.(*netFD).Read()
      C:/hostedtoolcache/windows/go/1.26.0/x64/src/net/fd_posix.go:68 +0x4a
  net.(*conn).Read()
      C:/hostedtoolcache/windows/go/1.26.0/x64/src/net/net.go:196 +0xac
  net/http.(*persistConn).Read()
      C:/hostedtoolcache/windows/go/1.26.0/x64/src/net/http/transport.go:2174 +0x232
  bufio.(*Reader).fill()
      C:/hostedtoolcache/windows/go/1.26.0/x64/src/bufio/bufio.go:113 +0x2a3
  bufio.(*Reader).Peek()
      C:/hostedtoolcache/windows/go/1.26.0/x64/src/bufio/bufio.go:152 +0xaf
  net/http.(*persistConn).readLoop()
      C:/hostedtoolcache/windows/go/1.26.0/x64/src/net/http/transport.go:2330 +0x31a
  net/http.(*Transport).dialConn.gowrap2()
      C:/hostedtoolcache/windows/go/1.26.0/x64/src/net/http/transport.go:1994 +0x2e

Previous write at 0x00c005271b40 by goroutine 8857:
  internal/poll.(*FD).setOffset()
      C:/hostedtoolcache/windows/go/1.26.0/x64/src/internal/poll/fd_windows.go:402 +0x6b9
  internal/poll.SendFile()
      C:/hostedtoolcache/windows/go/1.26.0/x64/src/internal/poll/sendfile_windows.go:71 +0x697
  net.sendFile.func1()
      C:/hostedtoolcache/windows/go/1.26.0/x64/src/net/sendfile.go:48 +0x64
  internal/poll.(*FD).RawRead()
      C:/hostedtoolcache/windows/go/1.26.0/x64/src/internal/poll/fd_windows.go:1275 +0x1ad
  os.(*rawConn).Read()
      C:/hostedtoolcache/windows/go/1.26.0/x64/src/os/rawconn.go:31 +0xb2
  net.sendFile()
      C:/hostedtoolcache/windows/go/1.26.0/x64/src/net/sendfile.go:47 +0x3cd
  net.(*TCPConn).readFrom()
      C:/hostedtoolcache/windows/go/1.26.0/x64/src/net/tcpsock_posix.go:51 +0x5b
  net.(*TCPConn).ReadFrom()
      C:/hostedtoolcache/windows/go/1.26.0/x64/src/net/tcpsock.go:165 +0x64
  io.copyBuffer()
      C:/hostedtoolcache/windows/go/1.26.0/x64/src/io/io.go:415 +0x20e
  io.Copy()
      C:/hostedtoolcache/windows/go/1.26.0/x64/src/io/io.go:388 +0x95
  net/http.persistConnWriter.ReadFrom()
      C:/hostedtoolcache/windows/go/1.26.0/x64/src/net/http/transport.go:2019 +0x12
  bufio.(*Writer).ReadFrom()
      C:/hostedtoolcache/windows/go/1.26.0/x64/src/bufio/bufio.go:797 +0x361
  io.copyBuffer()
      C:/hostedtoolcache/windows/go/1.26.0/x64/src/io/io.go:415 +0x20e
  io.CopyBuffer()
      C:/hostedtoolcache/windows/go/1.26.0/x64/src/io/io.go:402 +0x8f
  net/http.(*transferWriter).doBodyCopy()
      C:/hostedtoolcache/windows/go/1.26.0/x64/src/net/http/transfer.go:417 +0x13a
  net/http.(*transferWriter).writeBody()
      C:/hostedtoolcache/windows/go/1.26.0/x64/src/net/http/transfer.go:372 +0x764
  net/http.(*Request).write()
      C:/hostedtoolcache/windows/go/1.26.0/x64/src/net/http/request.go:762 +0x1376
  net/http.(*persistConn).writeLoop()
      C:/hostedtoolcache/windows/go/1.26.0/x64/src/net/http/transport.go:2655 +0x35d
  net/http.(*Transport).dialConn.gowrap3()
      C:/hostedtoolcache/windows/go/1.26.0/x64/src/net/http/transport.go:1995 +0x2e

Goroutine 8856 (running) created at:
  net/http.(*Transport).dialConn()
      C:/hostedtoolcache/windows/go/1.26.0/x64/src/net/http/transport.go:1994 +0x2fb0
  net/http.(*Transport).dialConnFor()
      C:/hostedtoolcache/windows/go/1.26.0/x64/src/net/http/transport.go:1648 +0x124
  net/http.(*Transport).startDialConnForLocked.func1()
      C:/hostedtoolcache/windows/go/1.26.0/x64/src/net/http/transport.go:1629 +0x3c

Goroutine 8857 (running) created at:
  net/http.(*Transport).dialConn()
      C:/hostedtoolcache/windows/go/1.26.0/x64/src/net/http/transport.go:1995 +0x303c
  net/http.(*Transport).dialConnFor()
      C:/hostedtoolcache/windows/go/1.26.0/x64/src/net/http/transport.go:1648 +0x124
  net/http.(*Transport).startDialConnForLocked.func1()
      C:/hostedtoolcache/windows/go/1.26.0/x64/src/net/http/transport.go:1629 +0x3c
==================
--- FAIL: TestRepositoriesService_DeleteFile (0.02s)
    testing.go:1712: race detected during execution of test
--- FAIL: TestRepositoriesService_TestHook (0.03s)
    testing.go:1712: race detected during execution of test
--- FAIL: TestRepositoriesService_RedeliverHookDelivery (0.03s)
    testing.go:1712: race detected during execution of test
--- FAIL: TestRepositoriesService_UploadReleaseAsset (0.32s)
    testing.go:1712: race detected during execution of test

Signed-off-by: Glenn Lewis <6598971+gmlewis@users.noreply.github.com>
@gmlewis gmlewis added the NeedsReview PR is awaiting a review before merging. label Feb 27, 2026
@codecov
Copy link

codecov bot commented Feb 27, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 94.08%. Comparing base (eabf610) to head (1db12de).

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #4051   +/-   ##
=======================================
  Coverage   94.08%   94.08%           
=======================================
  Files         207      207           
  Lines       19217    19220    +3     
=======================================
+ Hits        18081    18084    +3     
  Misses        938      938           
  Partials      198      198           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Contributor

@stevehipwell stevehipwell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The change looks good and the logic makes sense. Is there an upstream issue tracking this behaviour?

@gmlewis
Copy link
Collaborator Author

gmlewis commented Feb 27, 2026

The change looks good and the logic makes sense. Is there an upstream issue tracking this behaviour?

No. Must I make one? 😂

@gmlewis gmlewis removed the NeedsReview PR is awaiting a review before merging. label Feb 27, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants