data race in worker/uniter/runner/debug tests

Bug #1881857 reported by Tim Penhey
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Canonical Juju
Triaged
Low
Unassigned

Bug Description

Any single test run seems to be fine. Looking at the code it is possible that the goroutine started in server.go around line 81 is still running when another test is trying to patch out the waitClientExit function.

The fix is probably to add synchronization function that is empty by default and can be used in tests to add a sync point. However I'm a little unsure that even with a sync point in a defer function call will be sufficient to avoid this type of race.

WARNING: DATA RACE
Write at 0x00000151bbd8 by goroutine 78:
  reflect.Value.Set()
      /snap/go/5830/src/reflect/value.go:1536 +0x165
  github.com/juju/testing.PatchValue()
      /workspace/_build/src/github.com/juju/juju/vendor/github.com/juju/testing/patch.go:46 +0x17d
  github.com/juju/testing.(*CleanupSuite).PatchValue()
      /workspace/_build/src/github.com/juju/juju/vendor/github.com/juju/testing/cleanup.go:125 +0x77
  github.com/juju/juju/worker/uniter/runner/debug.(*DebugHooksServerSuite).TestRunHookExceptional()
      /workspace/_build/src/github.com/juju/juju/worker/uniter/runner/debug/server_test.go:126 +0x37f
  runtime.call32()
      /snap/go/5830/src/runtime/asm_amd64.s:539 +0x3a
  reflect.Value.Call()
      /snap/go/5830/src/reflect/value.go:321 +0xd3
  gopkg.in/check%2ev1.(*suiteRunner).forkTest.func1()
      /workspace/_build/src/github.com/juju/juju/vendor/gopkg.in/check.v1/check.go:781 +0xa0a
  gopkg.in/check%2ev1.(*suiteRunner).forkCall.func1()
      /workspace/_build/src/github.com/juju/juju/vendor/gopkg.in/check.v1/check.go:675 +0xd9

Previous read at 0x00000151bbd8 by goroutine 68:
  github.com/juju/juju/worker/uniter/runner/debug.(*ServerSession).RunHook.func2()
      /workspace/_build/src/github.com/juju/juju/worker/uniter/runner/debug/server.go:85 +0x3e

Goroutine 78 (running) created at:
  gopkg.in/check%2ev1.(*suiteRunner).forkCall()
      /workspace/_build/src/github.com/juju/juju/vendor/gopkg.in/check.v1/check.go:672 +0x44d
  gopkg.in/check%2ev1.(*suiteRunner).forkTest()
      /workspace/_build/src/github.com/juju/juju/vendor/gopkg.in/check.v1/check.go:763 +0x1b9
  gopkg.in/check%2ev1.(*suiteRunner).runTest()
      /workspace/_build/src/github.com/juju/juju/vendor/gopkg.in/check.v1/check.go:818 +0x228
  gopkg.in/check%2ev1.(*suiteRunner).run()
      /workspace/_build/src/github.com/juju/juju/vendor/gopkg.in/check.v1/check.go:624 +0x1c7
  gopkg.in/check%2ev1.Run()
      /workspace/_build/src/github.com/juju/juju/vendor/gopkg.in/check.v1/run.go:92 +0x5a
  gopkg.in/check%2ev1.RunAll()
      /workspace/_build/src/github.com/juju/juju/vendor/gopkg.in/check.v1/run.go:84 +0x127
  gopkg.in/check%2ev1.TestingT()
      /workspace/_build/src/github.com/juju/juju/vendor/gopkg.in/check.v1/run.go:72 +0x5f6
  github.com/juju/juju/worker/uniter/runner/debug_test.TestPackage()
      /workspace/_build/src/github.com/juju/juju/worker/uniter/runner/debug/package_test.go:13 +0x38
  testing.tRunner()
      /snap/go/5830/src/testing/testing.go:991 +0x1eb

Goroutine 68 (running) created at:
  github.com/juju/juju/worker/uniter/runner/debug.(*ServerSession).RunHook()
      /workspace/_build/src/github.com/juju/juju/worker/uniter/runner/debug/server.go:81 +0x58c
  github.com/juju/juju/worker/uniter/runner/debug.(*DebugHooksServerSuite).TestRunHookDebugAt.func1()
      /workspace/_build/src/github.com/juju/juju/worker/uniter/runner/debug/server_test.go:263 +0xcf
==================

Revision history for this message
Joseph Phillips (manadart) wrote :

This looks like:
https://bugs.launchpad.net/juju/+bug/1878130

Despite what it says there, the fix is committed to develop and 2.8, not 2.8-rc. I will amend.

Revision history for this message
Canonical Juju QA Bot (juju-qa-bot) wrote :

This bug has not been updated in 2 years, so we're marking it Low importance. If you believe this is incorrect, please update the importance.

Changed in juju:
importance: Medium → Low
tags: added: expirebugs-bot
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.