capnslog: fix small race on PackageLogger.level

[mischief]

WARNING: DATA RACE
Read by goroutine 46:
  github.com/coreos/pkg/capnslog.(*PackageLogger).internalLog()
      /home/mischief/code/go/src/github.com/coreos/pkg/capnslog/pkg_logger.go:30 +0x41
  github.com/coreos/pkg/capnslog.(*PackageLogger).Info()
      /home/mischief/code/go/src/github.com/coreos/pkg/capnslog/pkg_logger.go:131 +0x5a
...

Previous write by goroutine 44:
  github.com/coreos/pkg/capnslog.RepoLogger.setRepoLogLevelInternal()
      /home/mischief/code/go/src/github.com/coreos/pkg/capnslog/logmap.go:169 +0xba
  github.com/coreos/pkg/capnslog.SetGlobalLogLevel()
      /home/mischief/code/go/src/github.com/coreos/pkg/capnslog/logmap.go:136 +0x153
...

[vcaputo]

Reader-writer locks don't generally make sense where the critical section is trivially short like the single integer assignment being done here, since internally a rwmutex is still serializing the potentially concurrent read-lock acquisitions like a mutex would @ entry to the critical section internally either using atomics or a lock to manage the reader/writer counts. Basically the cost of maintaining the reader-writer mutex is comparable to the critical section itself, in this case. This situation is better served by an atomic, in lieu of going that route just use a simple mutex.

[mischief]

@vcaputo i think my vote currently would be plain mutex if that's preferred. LogLevel is a int8 type, and sync/atomic works on 32/64bit integers and i don't know if i can get it to cope with a int8. i'd have to change LogLevel's type, which seems like a poor idea, with consideration to all the users of this package.

[vcaputo]

sgtm!

[mischief]

switched to sync.Mutex, ptal

[vcaputo]

Is there any reason not to just piggyback on logger.Lock() for serializing the accesses to PackageLogger.level?
I don't think we need to worry about lock contention here

[vcaputo]

Line #46 below contains an unprotected access to the level member.

[mischief]

ptal

cc @vcaputo @barakmich

[crawford]

LGTM