pg-commands icon indicating copy to clipboard operation
pg-commands copied to clipboard

Race condition in Dump.Exec

Open retanik opened this issue 1 year ago • 2 comments

I encountered a race condition detected from go test and after reviewing your CircleCI Orb i see you have it turned off. It might be worth enabling it to catch those cases.

Orb Source line 312+

- go/test:
                covermode: atomic
                coverprofile: coverage.out
                failfast: true
                race: false
                verbose: true

Race condition was encountered between

Read at 0x00c0003fc020 by goroutine 67:
  github.com/habx/pg-commands.(*Dump).Exec()
      /x/x/go/pkg/mod/github.com/habx/[email protected]/pg_dump.go:64 +0xb16

and

Previous write at 0x00c0003fc020 by goroutine 68:
  github.com/habx/pg-commands.streamOutput()
      /x/x/go/pkg/mod/github.com/habx/[email protected]/utils.go:45 +0x1d3
  github.com/habx/pg-commands.(*Dump).Exec.func1()
      /x/x/go/pkg/mod/github.com/habx/[email protected]/pg_dump.go:54 +0x8a

I was running a unit test which uses suite.run to sequentially trigger multiple database exports. In Exec the streamOutput function is started as a go routine and can not be observed from outside to know when it has finished. I believe that is where the race condition occurs as it is unmanaged writing and reading from the Result.Error variable in both routines

retanik avatar Oct 31 '23 10:10 retanik

hi @retanik !

I can try your orb options in specific branch.

I can't reproduce race condition with your informations.

Can you give me :

  • code example
  • more logs
  • inputs informations

if you want, you can push a PR for fix issue.

clementlecorre avatar Nov 03 '23 11:11 clementlecorre

My estimate is that if you set race: true in your orb file on line 315. It will configure go/test to run with race conditions checking.

Exec and streamOutput both access the same variable result which i believe results in this race condition.

image

result.error can be manipulated by the streamOutput at the same time it is read to return from the Exec function. Because the timing is not guaranteed and the result variable has no locking through any kind of mutex for example, this will trigger the race condition warning.

hopefully this helps enough to reproduce

retanik avatar Feb 08 '24 12:02 retanik