Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

runtime/race: second_test_passes failures #50612

Closed
aclements opened this issue Jan 14, 2022 · 5 comments
Closed

runtime/race: second_test_passes failures #50612

aclements opened this issue Jan 14, 2022 · 5 comments
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone
Go1.18

Comments

@aclements
Copy link
Member

What version of Go are you using (go version)?

Commit 1abe9c1

What operating system and processor architecture are you using (go env)?

Detected on the freebsd-amd64-12_2 and freebsd-amd64-12_3 builders, but I don't think it's specific to those.

What did you do?

I ran all.bash 1,810 times between the freebsd-amd64-12_2 and freebsd-amd64-12_3 builders.

What did you expect to see?

Everything passes.

What did you see instead?

I got the following failure 8 times (0.0044% failure rate). If I'm reading this correctly, the problem is just that it expects TestFail to happen in < 1 second and it takes more than 1 second (in this example 1.54s, but I have examples ranging from 1.09s to 3.00s).

##### Testing race detector
--- FAIL: TestOutput (11.27s)
    output_test.go:30: test midstack_inlining_traceback runs only on linux, skipping:
    output_test.go:30: test external_cgo_thread runs only on linux, skipping:
    output_test.go:83: failed test case second_test_passes, expect:

        ==================
        --- FAIL: TestFail \(0...s\)
        .*testing.go:.*: race detected during execution of test
        FAIL
        got:
        ==================
        WARNING: DATA RACE
        Write at 0x00c00001a2d8 by goroutine 8:
          command-line-arguments_test.TestFail.func1()
              /tmp/workdir/tmp/TestOutput4792514/008/main_test.go:9 +0x39

        Previous write at 0x00c00001a2d8 by goroutine 7:
          command-line-arguments_test.TestFail()
              /tmp/workdir/tmp/TestOutput4792514/008/main_test.go:12 +0x118
          testing.tRunner()
              /tmp/workdir/go/src/testing/testing.go:1440 +0x213
          testing.(*T).Run.func1()
              /tmp/workdir/go/src/testing/testing.go:1487 +0x47

        Goroutine 8 (running) created at:
          command-line-arguments_test.TestFail()
              /tmp/workdir/tmp/TestOutput4792514/008/main_test.go:8 +0x10e
          testing.tRunner()
              /tmp/workdir/go/src/testing/testing.go:1440 +0x213
          testing.(*T).Run.func1()
              /tmp/workdir/go/src/testing/testing.go:1487 +0x47

        Goroutine 7 (running) created at:
          testing.(*T).Run()
              /tmp/workdir/go/src/testing/testing.go:1487 +0x724
          testing.runTests.func1()
              /tmp/workdir/go/src/testing/testing.go:1840 +0x99
          testing.tRunner()
              /tmp/workdir/go/src/testing/testing.go:1440 +0x213
          testing.runTests()
              /tmp/workdir/go/src/testing/testing.go:1838 +0x7e4
          testing.(*M).Run()
              /tmp/workdir/go/src/testing/testing.go:1720 +0xa71
          main.main()
              _testmain.go:49 +0x2e4
        ==================
        --- FAIL: TestFail (1.54s)
            testing.go:1313: race detected during execution of test
        FAIL
        FAIL    command-line-arguments  1.699s
        FAIL
FAIL
FAIL    runtime/race    11.295s
@aclements aclements added the NeedsFix The path to resolution is known, but the work has not been done. label Jan 14, 2022
@aclements aclements added this to the Go1.18 milestone Jan 14, 2022
@gopherbot
Copy link

Change https://golang.org/cl/378581 mentions this issue: runtime/race: be less picky about test run time

@zhiqiangxu
Copy link
Contributor

@aclements could you take a look at this? Sorry I didn't find another way to contact with you:(

https://forum.golangbridge.org/t/about-lemma-2-of-hybrid-write-barrier/26253

@aclements
Copy link
Member Author

@zhiqiangxu , that proof could be stated better. A stack is considered a single object for the purposes of marking. Stacks can't point to other stacks, and the heap can't point to a stack. So, B is either 1. a heap object, and the rest of the path has to be heap objects, or 2. it's stack, and all pointers from a stack either 2a. point to the heap, or 2b. point back to the same stack, in which case it's not a simple path and thus doesn't satisfy the precondition given on the path.

@ianlancetaylor
Copy link
Contributor

(Perhaps worth noting that there is at least one very special case in which the heap can point to a stack: channel operations in which the channel structure in the heap can point to the stack of a blocked goroutine.)

@zhiqiangxu
Copy link
Contributor

zhiqiangxu commented Jan 29, 2022 •
edited

@zhiqiangxu , that proof could be stated better. A stack is considered a single object for the purposes of marking. Stacks can't point to other stacks, and the heap can't point to a stack. So, B is either 1. a heap object, and the rest of the path has to be heap objects, or 2. it's stack, and all pointers from a stack either 2a. point to the heap, or 2b. point back to the same stack, in which case it's not a simple path and thus doesn't satisfy the precondition given on the path.

@aclements Thanks for clarification, it makes much more sense now!

But as @ianlancetaylor stated above, heap can also point to a stack for the channel?

jproberts pushed a commit to jproberts/go that referenced this issue Jun 21, 2022
@aclements @jproberts
runtime/race: be less picky about test run time
e60741f 
Currently, there are two regexps in the race detector output tests
that assume subtests will complete in < 1 second. This isn't necessary
and very occasionally fails (on builders that are probably very
loaded). Make these tests less picky about timing.

Fixes golang#50612.

Change-Id: Ib3f94d6c5dc37541dbeb06de71cf462a74af844b
Reviewed-on: https://go-review.googlesource.com/c/go/+/378581
Trust: Austin Clements <austin@google.com>
Run-TryBot: Austin Clements <austin@google.com>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
@golang golang locked and limited conversation to collaborators Jan 29, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Milestone
Go1.18
Development

No branches or pull requests
4 participants
@zhiqiangxu @aclements @ianlancetaylor @gopherbot