Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Race condition in Dump.Exec #53

Open
retanik opened this issue Oct 31, 2023 · 2 comments
Open

Race condition in Dump.Exec #53

retanik opened this issue Oct 31, 2023 · 2 comments

Comments

@retanik
Copy link

retanik commented Oct 31, 2023

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

@clementlecorre
Copy link
Member

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.

@retanik
Copy link
Author

retanik commented Feb 8, 2024

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

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

No branches or pull requests

2 participants