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

runtime: TestFinalizerInterfaceBig failures #57166

Closed
gopherbot opened this issue Dec 8, 2022 · 5 comments
Closed

runtime: TestFinalizerInterfaceBig failures #57166

gopherbot opened this issue Dec 8, 2022 · 5 comments
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. Testing An issue that has been verified to require only test changes, not just a test failure.
Milestone

Comments

@gopherbot
Copy link

#!watchflakes
post <- pkg == "runtime" && test == "TestFinalizerInterfaceBig"

Issue created automatically to collect these failures.

Example (log):

--- FAIL: TestFinalizerInterfaceBig (4.00s)
    mfinal_test.go:115: finalizer for type *bigValue didn't run

watchflakes

@gopherbot gopherbot added NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. compiler/runtime Issues related to the Go compiler and/or runtime. labels Dec 8, 2022
@gopherbot
Copy link
Author

Found new dashboard test flakes for:

#!watchflakes
post <- pkg == "runtime" && test == "TestFinalizerInterfaceBig"
2022-12-08 03:49 windows-amd64-2016 go@b9747e0e runtime.TestFinalizerInterfaceBig (log)
--- FAIL: TestFinalizerInterfaceBig (4.00s)
    mfinal_test.go:115: finalizer for type *bigValue didn't run

watchflakes

@bcmills
Copy link
Contributor

bcmills commented Dec 8, 2022

This test uses an arbitrary timeout and a single GC cycle:

https://cs.opensource.google/go/go/+/master:src/runtime/mfinal_test.go;l=114;drc=58804ea67a28c1d8e37ed548b685bc0c09638886

On a heavily-loaded builder, that could cause it to fail spuriously due to a system scheduler delay.

@bcmills bcmills added Testing An issue that has been verified to require only test changes, not just a test failure. NeedsFix The path to resolution is known, but the work has not been done. labels Dec 8, 2022
@gopherbot gopherbot removed the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Dec 8, 2022
@bcmills
Copy link
Contributor

bcmills commented Dec 8, 2022

Interestingly, this has only failed twice in the past ~2 years — both times recently on Windows.

greplogs -l -e 'FAIL: TestFinalizer' --since=2021-01-01
2022-12-08T03:49:48-b9747e0/windows-amd64-2016
2022-08-08T15:22:02-dd59088/windows-amd64-longtest-newcc

@gopherbot
Copy link
Author

Change https://go.dev/cl/456118 mentions this issue: runtime: remove arbitrary timeouts in finalizer tests

@gopherbot
Copy link
Author

Change https://go.dev/cl/456120 mentions this issue: runtime: remove arbitrary GOARCH constraints in finalizer tests

@dmitshur dmitshur added this to the Go1.20 milestone Dec 8, 2022
gopherbot pushed a commit that referenced this issue Jan 19, 2023
These tests were only run on GOARCH=amd64, but the rationale given in
CL 11858043 was GC precision on 32-bit platforms. Today, we have far
more 64-bit platforms than just amd64, and I believe that GC precision
on 32-bit platforms has been substantially improved as well.
The GOARCH restriction seems unnecessary.

Updates #57166.
Updates #5368.

Change-Id: I45c608b6fa721012792c96d4ed94a6d772b90210
Reviewed-on: https://go-review.googlesource.com/c/go/+/456120
Run-TryBot: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Bryan Mills <bcmills@google.com>
Reviewed-by: Austin Clements <austin@google.com>
@golang golang locked and limited conversation to collaborators Dec 8, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. Testing An issue that has been verified to require only test changes, not just a test failure.
Projects
Status: Done
Development

No branches or pull requests

3 participants