gocheck: serialize log writes
Fixes LP #1084878.
The race detector cannot profile gocheck tested code due to this race.
R=rog, niemeyer, fwereade
CC=
https://codereview.appspot.com/6862050
diff --git a/gocheck.go b/gocheck.go
index c353b3e..542e2eb 100644
--- a/gocheck.go
+++ b/gocheck.go
@@ -72,15 +72,16 @@
}
type C struct {
- method *methodType
- kind funcKind
- status funcStatus
- logb *bytes.Buffer
- logw io.Writer
- done chan *C
- reason string
- mustFail bool
- tempDir *tempDir
+ method *methodType
+ kind funcKind
+ status funcStatus
+ writeLock sync.Mutex // protects logb from concurrent writes
+ logb *logger
+ logw io.Writer
+ done chan *C
+ reason string
+ mustFail bool
+ tempDir *tempDir
timer
}
@@ -88,6 +89,30 @@
runtime.Goexit()
}
+// logger concurrency safe byte.Buffer
+type logger struct {
+ sync.Mutex
+ writer bytes.Buffer
+}
+
+func (l *logger) Write(buf []byte) (int, error) {
+ l.Lock()
+ defer l.Unlock()
+ return l.writer.Write(buf)
+}
+
+func (l *logger) WriteTo(w io.Writer) (int64, error) {
+ l.Lock()
+ defer l.Unlock()
+ return l.writer.WriteTo(w)
+}
+
+func (l *logger) String() string {
+ l.Lock()
+ defer l.Unlock()
+ return l.writer.String()
+}
+
// -----------------------------------------------------------------------
// Handling of temporary files and directories.
@@ -580,13 +605,13 @@
// Create a call object with the given suite method, and fork a
// goroutine with the provided dispatcher for running it.
-func (runner *suiteRunner) forkCall(method *methodType, kind funcKind, logb *bytes.Buffer, dispatcher func(c *C)) *C {
+func (runner *suiteRunner) forkCall(method *methodType, kind funcKind, logb *logger, dispatcher func(c *C)) *C {
var logw io.Writer
if runner.output.Stream {
logw = runner.output
}
if logb == nil {
- logb = bytes.NewBuffer(nil)
+ logb = new(logger)
}
c := &C{
method: method,
@@ -607,7 +632,7 @@
}
// Same as forkCall(), but wait for call to finish before returning.
-func (runner *suiteRunner) runFunc(method *methodType, kind funcKind, logb *bytes.Buffer, dispatcher func(c *C)) *C {
+func (runner *suiteRunner) runFunc(method *methodType, kind funcKind, logb *logger, dispatcher func(c *C)) *C {
c := runner.forkCall(method, kind, logb, dispatcher)
<-c.done
return c
@@ -650,7 +675,7 @@
// goroutine like all suite methods, but this method will not return
// while the fixture goroutine is not done, because the fixture must be
// run in a desired order.
-func (runner *suiteRunner) runFixture(method *methodType, logb *bytes.Buffer) *C {
+func (runner *suiteRunner) runFixture(method *methodType, logb *logger) *C {
if method != nil {
c := runner.runFunc(method, fixtureKd, logb, func(c *C) {
c.ResetTimer()
@@ -666,7 +691,7 @@
// Run the fixture method with runFixture(), but panic with a fixturePanic{}
// in case the fixture method panics. This makes it easier to track the
// fixture panic together with other call panics within forkTest().
-func (runner *suiteRunner) runFixtureWithPanic(method *methodType, logb *bytes.Buffer, skipped *bool) *C {
+func (runner *suiteRunner) runFixtureWithPanic(method *methodType, logb *logger, skipped *bool) *C {
if *skipped {
return nil
}
diff --git a/helpers_test.go b/helpers_test.go
index 79348bc..fb9ef8e 100644
--- a/helpers_test.go
+++ b/helpers_test.go
@@ -7,6 +7,8 @@
"launchpad.net/gocheck"
"os"
"reflect"
+ "runtime"
+ "sync"
)
var helpersS = gocheck.Suite(&HelpersS{})
@@ -428,6 +430,26 @@
return false
}
+// Concurrent logging should not corrupt the underling buffer.
+// Use go test -race to detect the race in this test.
+func (s *HelpersS) TestConcurrentLogging(c *gocheck.C) {
+ defer runtime.GOMAXPROCS(runtime.GOMAXPROCS(runtime.NumCPU()))
+ var start, stop sync.WaitGroup
+ start.Add(1)
+ for i, n := 0, runtime.NumCPU()*2; i < n; i++ {
+ stop.Add(1)
+ go func(i int) {
+ start.Wait()
+ for j := 0; j < 30; j++ {
+ c.Logf("Worker %d: line %d", i, j)
+ }
+ stop.Done()
+ }(i)
+ }
+ start.Done()
+ stop.Wait()
+}
+
// -----------------------------------------------------------------------
// A couple of helper functions to test helper functions. :-)
