Problem with Race Conditions

[albertoleal]

Hi there! I'm doing some tests with this package but it seems that there's any issue when dealing with race conditions, not sure. Any advice?

func (storage *Storage) GetTokenValue(key string, t interface{}) error {
    var (
        data []interface{}
        err  error
    )

    if item := Cache.Get(key); item != nil {
        if !item.Expired() {
            data = item.Value().([]interface{})
        }
    }
    if len(data) == 0 {
        data, err = getCacheFromRedis(key)
        if err != nil {
            return err
        }
    }

    if err = redis.ScanStruct(data, t); err != nil {
        return err
    }
    Cache.Set(key, data, time.Duration(10)*time.Minute)
    return nil
}

And, when I run go test -race -i ./... I get this warning:

==================
WARNING: DATA RACE
Read by goroutine 20:
  sync/atomic.AddUint32()
      /usr/local/Cellar/go/1.3.3/libexec/src/pkg/sync/atomic/race.go:147 +0x4e
  sync/atomic.AddInt32()
      /usr/local/Cellar/go/1.3.3/libexec/src/pkg/sync/atomic/race.go:140 +0x3c
  github.com/karlseguin/ccache.(*Item).shouldPromote()
      /Users/alberto/Code/golang/src/github.com/karlseguin/ccache/item.go:70 +0x48
  github.com/karlseguin/ccache.(*Cache).conditionalPromote()
      /Users/alberto/Code/golang/src/github.com/karlseguin/ccache/cache.go:138 +0x67
  github.com/karlseguin/ccache.(*Cache).Get()
      /Users/alberto/Code/golang/src/github.com/karlseguin/ccache/cache.go:52 +0xfc
  github.com/backstage/backstage/db.(*Storage).GetTokenValue()
      /Users/alberto/Code/golang/src/github.com/backstage/backstage/db/storage.go:77 +0xc0
  github.com/backstage/backstage/auth.get()
      /Users/alberto/Code/golang/src/github.com/backstage/backstage/auth/token.go:130 +0x174
  github.com/backstage/backstage/auth.RevokeTokensFor()
      /Users/alberto/Code/golang/src/github.com/backstage/backstage/auth/token.go:93 +0x146
  github.com/backstage/backstage/auth.(*S).TestRevokeTokensFor()
      /Users/alberto/Code/golang/src/github.com/backstage/backstage/auth/token_test.go:54 +0x20b
  runtime.call16()
      /usr/local/Cellar/go/1.3.3/libexec/src/pkg/runtime/asm_amd64.s:360 +0x31
  reflect.Value.Call()
      /usr/local/Cellar/go/1.3.3/libexec/src/pkg/reflect/value.go:411 +0xed
  gopkg.in/check%2ev1.func·003()
      /Users/alberto/Code/golang/src/gopkg.in/check.v1/check.go:763 +0x56b
  gopkg.in/check%2ev1.func·001()
      /Users/alberto/Code/golang/src/gopkg.in/check.v1/check.go:657 +0xf4

Previous write by goroutine 7:
  github.com/karlseguin/ccache.(*Cache).doPromote()
      /Users/alberto/Code/golang/src/github.com/karlseguin/ccache/cache.go:171 +0x64
  github.com/karlseguin/ccache.(*Cache).worker()
      /Users/alberto/Code/golang/src/github.com/karlseguin/ccache/cache.go:152 +0xae

Goroutine 20 (running) created at:
  gopkg.in/check%2ev1.(*suiteRunner).forkCall()
      /Users/alberto/Code/golang/src/gopkg.in/check.v1/check.go:658 +0x523
  gopkg.in/check%2ev1.(*suiteRunner).forkTest()
      /Users/alberto/Code/golang/src/gopkg.in/check.v1/check.go:795 +0x168
  gopkg.in/check%2ev1.(*suiteRunner).runTest()
      /Users/alberto/Code/golang/src/gopkg.in/check.v1/check.go:800 +0x3e
  gopkg.in/check%2ev1.(*suiteRunner).run()
      /Users/alberto/Code/golang/src/gopkg.in/check.v1/check.go:606 +0x4e8
  gopkg.in/check%2ev1.Run()
      /Users/alberto/Code/golang/src/gopkg.in/check.v1/run.go:92 +0x56
  gopkg.in/check%2ev1.RunAll()
      /Users/alberto/Code/golang/src/gopkg.in/check.v1/run.go:84 +0x12d
  gopkg.in/check%2ev1.TestingT()
      /Users/alberto/Code/golang/src/gopkg.in/check.v1/run.go:72 +0x4f1
  github.com/backstage/backstage/auth.Test()
      /Users/alberto/Code/golang/src/github.com/backstage/backstage/auth/suite_test.go:10 +0x34
  testing.tRunner()
      /usr/local/Cellar/go/1.3.3/libexec/src/pkg/testing/testing.go:422 +0x10f

Goroutine 7 (running) created at:
  github.com/karlseguin/ccache.New()
      /Users/alberto/Code/golang/src/github.com/karlseguin/ccache/cache.go:37 +0x38e
  github.com/backstage/backstage/db.init()
      /Users/alberto/Code/golang/src/github.com/backstage/backstage/db/cache.go:5 +0xc3
  github.com/backstage/backstage/auth.init()
      /Users/alberto/Code/golang/src/github.com/backstage/backstage/auth/token_test.go:57 +0xac
  main.init()
      github.com/backstage/backstage/auth/_test/_testmain.go:48 +0x93
==================

[karlseguin]

Odd I get no race reports on my computer, but you / it is correct.

557d56ec6f5b386def495b298667858d837046eb hopefully fixes this. The concurrency model has been simplified and only the worker accesses item.promotions.

Similarly, 6df1e24ae3dec3f6932c4e5e46aa7252ee5f87ca makes it so that cache.size is now only accessed by the worker as well.

The two changes simplify the flow and reduce contention. I'll leave this open for a bit in case someone wants to review it :)

[albertoleal]

@karlseguin Just run my test suite without problem. Thank you a lot for the quick fix and for this amazing project :D