globalsign / mgo

The MongoDB driver for Go
Other
1.97k stars 230 forks source link

Lazy initialization of coarse time goroutine #281

Closed cezarsa closed 6 years ago

cezarsa commented 6 years ago

This is a resubmission of #278 on the correct branch now.

Initializing the coarseTime goroutine in the init() call had a pretty serious side effect. It was impossible to override the value of time.Local without causing a race condition, because its value is used on NewTicker calls.

This commit changes the previous behavior by only initializing the coarseTime goroutine when the first server is created. Allowing programs to possibly override time.Local before mgo is initialized.

Previously the following code:

package main

import (
    "time"

    _ "github.com/globalsign/mgo"
)

func main() {
    time.Local = time.UTC
}

Running go run -race main.go causes the following output:

$ go run -race main.go
==================
WARNING: DATA RACE
Read at 0x000001310d98 by goroutine 7:
  time.Now()
      /usr/local/Cellar/go/1.11/libexec/src/time/time.go:1060 +0xcf
  time.sendTime()
      /usr/local/Cellar/go/1.11/libexec/src/time/sleep.go:141 +0x44

Previous write at 0x000001310d98 by main goroutine:
  main.main()
      /Users/cezarsa/go/src/github.com/tsuru/tsuru-client/main.go:10 +0x56

Goroutine 7 (running) created at:
  runtime.(*timersBucket).addtimerLocked()
      /usr/local/Cellar/go/1.11/libexec/src/runtime/time.go:170 +0x113
  github.com/tsuru/tsuru-client/vendor/github.com/globalsign/mgo.newcoarseTimeProvider.func1()
      /Users/cezarsa/go/src/github.com/tsuru/tsuru-client/vendor/github.com/globalsign/mgo/coarse_time.go:49 +0x50
==================
Found 1 data race(s)
exit status 66

@domodwyer: I've addressed your previous review points.

eminano commented 6 years ago

@cezarsa Thanks for resubmitting, and for your contribution :)