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

Prevent data race #36

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Prevent data race #36

wants to merge 2 commits into from

Conversation

k3a
Copy link

@k3a k3a commented Jun 20, 2019

This is finally the correct pull request preventing this data race (using race detector - go run -race ...):

==================
WARNING: DATA RACE
Write at 0x00c0000ba0c8 by goroutine 16:
  github.com/gofinance/ib.(*AbstractManager).startMainLoop.func2()
      /home/kexik/go/pkg/mod/github.com/gofinance/[email protected]/manager.go:118 +0x7d

Previous read at 0x00c0000ba0c8 by goroutine 14:
  github.com/gofinance/ib.(*AbstractManager).startMainLoop()
      /home/kexik/go/pkg/mod/github.com/gofinance/[email protected]/manager.go:126 +0x2b9

Goroutine 16 (running) created at:
  github.com/gofinance/ib.(*AbstractManager).startMainLoop()
      /home/kexik/go/pkg/mod/github.com/gofinance/[email protected]/manager.go:112 +0x1cd

Goroutine 14 (running) created at:
  github.com/gofinance/ib.NewHistoricalDataManager()
      /home/kexik/go/pkg/mod/github.com/gofinance/[email protected]/historical_data_manager.go:25 +0x499
==================

Talking about startMainLoop in manager.go:

  • Closing errors channel would cause CPU hog in the infinite for loop in startMainLoop (the case of the closed errors channel would be selected with ok=false every iteration).
  • Preventing that CPU hog by setting errors = nil in that other gorountine causes this data race

The solution is actually leaving errors channel living, that is not a problem, it will be garbage-collected after the for loop is broken by any of the other case and the exit from startMainLoop.

k3a added 2 commits June 19, 2019 23:48
Using second return value of receive operator satisfies the select's case when `errors` channel is closed, making another unnecessary for loop iteration.
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.

1 participant