Skip to content

bug: Kill() invoked shortly after Run() results in data races #1689

@lrstanley

Description

@lrstanley

Describe the bug
If you invoke Kill, shortly after invoking Run, it results in data races (shown below), in addition to some cases of fatal error: sync: unlock of unlocked mutex (couldn't reliably replicate). This could occur in a few scenarios, some examples:

  • Short-duration tests, where a program is spun up and shut down very quickly (using Kill to ensure cleanup, where sending Quit doesn't necessarily guarantee this).
  • Program's spun up from potentially remote connections, where someone could theoretically introduce a lot of connections that terminate quickly.

Setup
Please complete the following information along with version numbers, if applicable.

  • OS: n/a
  • Shell: n/a
  • Terminal Emulator: n/a
  • Terminal Multiplexer: n/a

To Reproduce
The following test reproduces:

func TestKillDuringStartupRace(t *testing.T) {
	for range 100 {
		p := NewProgram(&testModel{},
			WithInput(bytes.NewBuffer(nil)),
			WithOutput(io.Discard),
			WithoutSignals(),
		)

		done := make(chan struct{})
		go func() {
			_, _ = p.Run()
			close(done)
		}()

		p.Kill()

		select {
		case <-done:
		case <-time.After(time.Second):
			t.Fatal("program did not stop")
		}
	}
}

Expected behavior

Due to Run not really providing a way to know when things are "started enough", it's hard to know when or when not Kill should be safe to invoke. I would expect one of these two:

  1. Provide a way to know when it's safe to invoke Kill.
  2. Make Kill block until startup is safe enough to easily be cancelled/"unrolled". However, this still needs to ensure that it can't run into a scenario where it blocks forever.

Additional context

go test running with -race, showing the data race
$ go test -race -timeout 20s -count 20 -cpu 1,4 -run 'TestKillDuringStartupRace$' .
==================
WARNING: DATA RACE
Write at 0x00c0000b47b0 by goroutine 10:
  charm.land/bubbletea/v2.(*Program).Run()
      /home/liam/tmp/fork-bubbletea/tea.go:997 +0x87
  charm.land/bubbletea/v2.TestKillDuringStartupRace.func1()
      /home/liam/tmp/fork-bubbletea/tea_test.go:230 +0x33

Previous read at 0x00c0000b47b0 by goroutine 9:
  charm.land/bubbletea/v2.(*channelHandlers).shutdown()
      /home/liam/tmp/fork-bubbletea/tea.go:415 +0xd2
  charm.land/bubbletea/v2.TestKillDuringStartupRace.(*Program).Kill.(*Program).shutdown.func5()
      /home/liam/tmp/fork-bubbletea/tea.go:1246 +0x58
  sync.(*Once).doSlow()
      /usr/local/go/src/sync/once.go:78 +0xa1
  testing.tRunner()
      /usr/local/go/src/testing/testing.go:2036 +0x21c
  testing.(*T).Run.gowrap1()
      /usr/local/go/src/testing/testing.go:2101 +0x38

Goroutine 10 (running) created at:
  charm.land/bubbletea/v2.TestKillDuringStartupRace()
      /home/liam/tmp/fork-bubbletea/tea_test.go:229 +0x359
  testing.tRunner()
      /usr/local/go/src/testing/testing.go:2036 +0x21c
  testing.(*T).Run.gowrap1()
      /usr/local/go/src/testing/testing.go:2101 +0x38

Goroutine 9 (running) created at:
  testing.(*T).Run()
      /usr/local/go/src/testing/testing.go:2101 +0xb12
  testing.runTests.func1()
      /usr/local/go/src/testing/testing.go:2585 +0x85
  testing.tRunner()
      /usr/local/go/src/testing/testing.go:2036 +0x21c
  testing.runTests()
      /usr/local/go/src/testing/testing.go:2583 +0x9e9
  testing.(*M).Run()
      /usr/local/go/src/testing/testing.go:2443 +0xf4b
  main.main()
      _testmain.go:102 +0x164
==================
==================
WARNING: DATA RACE
Write at 0x00c0000b4888 by goroutine 10:
  charm.land/bubbletea/v2.(*Program).Run()
      /home/liam/tmp/fork-bubbletea/tea.go:1077 +0xc24
  charm.land/bubbletea/v2.TestKillDuringStartupRace.func1()
      /home/liam/tmp/fork-bubbletea/tea_test.go:230 +0x33

Previous read at 0x00c0000b4888 by goroutine 9:
  charm.land/bubbletea/v2.TestKillDuringStartupRace.(*Program).Kill.(*Program).shutdown.func5()
      /home/liam/tmp/fork-bubbletea/tea.go:1259 +0xf1
  sync.(*Once).doSlow()
      /usr/local/go/src/sync/once.go:78 +0xa1
  testing.tRunner()
      /usr/local/go/src/testing/testing.go:2036 +0x21c
  testing.(*T).Run.gowrap1()
      /usr/local/go/src/testing/testing.go:2101 +0x38

Goroutine 10 (running) created at:
  charm.land/bubbletea/v2.TestKillDuringStartupRace()
      /home/liam/tmp/fork-bubbletea/tea_test.go:229 +0x359
  testing.tRunner()
      /usr/local/go/src/testing/testing.go:2036 +0x21c
  testing.(*T).Run.gowrap1()
      /usr/local/go/src/testing/testing.go:2101 +0x38

Goroutine 9 (running) created at:
  testing.(*T).Run()
      /usr/local/go/src/testing/testing.go:2101 +0xb12
  testing.runTests.func1()
      /usr/local/go/src/testing/testing.go:2585 +0x85
  testing.tRunner()
      /usr/local/go/src/testing/testing.go:2036 +0x21c
  testing.runTests()
      /usr/local/go/src/testing/testing.go:2583 +0x9e9
  testing.(*M).Run()
      /usr/local/go/src/testing/testing.go:2443 +0xf4b
  main.main()
      _testmain.go:102 +0x164
==================
==================
WARNING: DATA RACE
Write at 0x00c0000b48e8 by goroutine 10:
  charm.land/bubbletea/v2.(*Program).initInputReader()
      /home/liam/tmp/fork-bubbletea/tty.go:69 +0x22c
  charm.land/bubbletea/v2.(*Program).Run()
      /home/liam/tmp/fork-bubbletea/tea.go:1101 +0x11a9
  charm.land/bubbletea/v2.TestKillDuringStartupRace.func1()
      /home/liam/tmp/fork-bubbletea/tea_test.go:230 +0x33

Previous read at 0x00c0000b48e8 by goroutine 9:
  charm.land/bubbletea/v2.TestKillDuringStartupRace.(*Program).Kill.(*Program).shutdown.func5()
      /home/liam/tmp/fork-bubbletea/tea.go:1249 +0x6d
  sync.(*Once).doSlow()
      /usr/local/go/src/sync/once.go:78 +0xa1
  testing.tRunner()
      /usr/local/go/src/testing/testing.go:2036 +0x21c
  testing.(*T).Run.gowrap1()
      /usr/local/go/src/testing/testing.go:2101 +0x38

Goroutine 10 (running) created at:
  charm.land/bubbletea/v2.TestKillDuringStartupRace()
      /home/liam/tmp/fork-bubbletea/tea_test.go:229 +0x359
  testing.tRunner()
      /usr/local/go/src/testing/testing.go:2036 +0x21c
  testing.(*T).Run.gowrap1()
      /usr/local/go/src/testing/testing.go:2101 +0x38

Goroutine 9 (running) created at:
  testing.(*T).Run()
      /usr/local/go/src/testing/testing.go:2101 +0xb12
  testing.runTests.func1()
      /usr/local/go/src/testing/testing.go:2585 +0x85
  testing.tRunner()
      /usr/local/go/src/testing/testing.go:2036 +0x21c
  testing.runTests()
      /usr/local/go/src/testing/testing.go:2583 +0x9e9
  testing.(*M).Run()
      /usr/local/go/src/testing/testing.go:2443 +0xf4b
  main.main()
      _testmain.go:102 +0x164
==================
--- FAIL: TestKillDuringStartupRace (0.08s)
    testing.go:1712: race detected during execution of test
FAIL
FAIL    charm.land/bubbletea/v2 12.195s
FAIL

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions