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.

Sign up for GitHub

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

testing: parallel fuzzing hangs with -parallel=1 #50217

Closed
AlekSi opened this issue Dec 16, 2021 · 8 comments
Closed

testing: parallel fuzzing hangs with -parallel=1 #50217

AlekSi opened this issue Dec 16, 2021 · 8 comments
Labels
FrozenDueToAge fuzz Issues related to native fuzzing support NeedsFix The path to resolution is known, but the work has not been done.
Milestone
Go1.18

Comments

@AlekSi
Copy link
Contributor

AlekSi commented Dec 16, 2021 •
edited 

go version devel go1.18-7f2314530e Thu Dec 16 00:34:10 2021 +0000 darwin/arm64

The following test hangs when run with go test -parallel=1:

func FuzzParallel(f *testing.F) {
	f.Add(42)

	f.Fuzz(func(t *testing.T, i int) {
		t.Parallel()
	})
}

It works with -parallel=2 and other values. I think it should work in any case.

Small self-contained example is there: https://github.com/AlekSi/go-bug/tree/master/50217
@AlekSi

This comment has been minimized.

Sign in to view
@gopherbot gopherbot added the fuzz Issues related to native fuzzing support label Dec 16, 2021
@AlekSi
Copy link
Contributor Author

AlekSi commented Dec 16, 2021

Bisected down to b2b6be7

@cherrymui
Copy link
Member

cc @golang/fuzzing

@cherrymui cherrymui added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Dec 16, 2021
@cherrymui cherrymui added this to the Go1.18 milestone Dec 16, 2021
@rolandshoemaker
Copy link
Member

This seems like one of the things that should just be disabled, both within the f.Fuzz function, and disallowing the flag, since I'm not really sure what it would do. @katiehockman what do you think?

@thepudds
Copy link
Contributor

FWIW, in terms of what it could do, the design doc includes:

// Run the fuzz test
f.Fuzz(func(t *testing.T, a string, num *big.Int) {
	t.Parallel() // seed corpus tests can run in parallel

Certainly seems reasonable to disallow it though, especially for a first version.

@AlekSi
Copy link
Contributor Author

AlekSi commented Dec 17, 2021

Running seed corpus entries in parallel actually works (as far as I can tell) and is very useful for me. If there is not enough time to fix it for -parallel=1 edge case, maybe just explicitly fail in that case instead of hanging or removing the whole feature.

@katiehockman
Copy link
Contributor

t.Parallel() in a fuzz target should be a no-op during fuzzing, but is similar to a t.Parallel() inside a t.Run() in that it runs the seed corpus in parallel when -fuzz isn't set.
This is just a bug I think. If we can't fix it, then just removing that functionality for 1.18 also seems fine. Will take a look.

@katiehockman katiehockman self-assigned this Dec 20, 2021
@katiehockman katiehockman added NeedsFix The path to resolution is known, but the work has not been done. and removed NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Dec 20, 2021
@gopherbot
Copy link

Change https://golang.org/cl/374054 mentions this issue: testing: fix deadlock with t.Parallel in testing seed corpus

jproberts pushed a commit to jproberts/go that referenced this issue Jun 21, 2022
@katiehockman @jproberts
testing: fix deadlock with t.Parallel in testing seed corpus
b2f067e 
The c.startParallel channel on the testContext is stuck
in t.Parallel() because c.running starts at 1 for the main
fuzz parent test, and is causing a deadlock because it is
never released. It would normally be released by tRunner,
but needs to instead be released by fRunner instead for fuzz
tests.

Fixes golang#50217

Change-Id: I2d010e9adddfd8e8321ff2f9dd2e43daf46c128f
Reviewed-on: https://go-review.googlesource.com/c/go/+/374054
Reviewed-by: Roland Shoemaker <roland@golang.org>
Reviewed-by: Bryan Mills <bcmills@google.com>
Trust: Katie Hockman <katie@golang.org>
Run-TryBot: Katie Hockman <katie@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
@golang golang locked and limited conversation to collaborators Jun 22, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge fuzz Issues related to native fuzzing support NeedsFix The path to resolution is known, but the work has not been done.
Projects
Go Security
Status: No status
Milestone
Go1.18
Development

No branches or pull requests
6 participants
@AlekSi @rolandshoemaker @gopherbot @cherrymui @thepudds @katiehockman