code review 6820098: runtime: fix deadlock in parallel for test

runtime: fix deadlock in parallel for test
The deadlock occurs when another goroutine requests GC
during the test. When wait=true the test expects physical parallelism,
that is, that P goroutines are all active at the same time.
If GC is requested, then part of the goroutines are not scheduled,
so other goroutines deadlock.
With wait=false, goroutines finish parallel for w/o waiting for all
other goroutines.
Fixes issue 3954.

[dvyukov, 2012-11-06 15:36:42]

Hello golang-dev@googlegroups.com,

I'd like you to review this change to
https://dvyukov%40google.com@code.google.com/p/go/

[bradfitz, 2012-11-06 16:03:11]

LGTM

I'd probably use a WaitGroup here, but this works too.

On Tue, Nov 6, 2012 at 4:36 PM, <dvyukov@google.com> wrote:

> Reviewers: golang-dev_googlegroups.com,
>
> Message:
> Hello golang-dev@googlegroups.com,
>
> I'd like you to review this change to
>
https://dvyukov%40google.com@**code.google.com/p/go/<http://40google.com@code...
>
>
> Description:
> runtime: fix deadlock in parallel for test
> The deadlock occurs when another goroutine requests GC
> during the test. When wait=true the test expects physical parallelism,
> that is, that P goroutines are all active at the same time.
> If GC is requested, then part of the goroutines are not scheduled,
> so other goroutines deadlock.
> With wait=false, goroutines finish parallel for w/o waiting for all
> other goroutines.
> Fixes issue 3954.
>
> Please review this at
http://codereview.appspot.com/**6820098/<http://codereview.appspot.com/6820098/>
>
> Affected files:
>   M src/pkg/runtime/parfor_test.go
>
>
> Index: src/pkg/runtime/parfor_test.go
> ==============================**==============================**=======
> --- a/src/pkg/runtime/parfor_test.**go
> +++ b/src/pkg/runtime/parfor_test.**go
> @@ -106,14 +106,21 @@
>                 data[i] = i
>         }
>         P := GOMAXPROCS(-1)
> +       c := make(chan bool, P)
>         desc := NewParFor(uint32(P))
> -       ParForSetup(desc, uint32(P), uint32(N), nil, true, func(desc
> *ParFor, i uint32) {
> +       ParForSetup(desc, uint32(P), uint32(N), nil, false, func(desc
> *ParFor, i uint32) {
>                 data[i] = data[i]*data[i] + 1
>         })
>         for p := 1; p < P; p++ {
> -               go ParForDo(desc)
> +               go func() {
> +                       ParForDo(desc)
> +                       c <- true
> +               }()
>         }
>         ParForDo(desc)
> +       for p := 1; p < P; p++ {
> +               <-c
> +       }
>         for i := uint64(0); i < N; i++ {
>                 if data[i] != i*i+1 {
>                         t.Fatalf("Wrong element %d: %d", i, data[i])
>
>
>

[dvyukov, 2012-11-06 16:11:38]

*** Submitted as http://code.google.com/p/go/source/detail?r=0852bd93e466 ***

runtime: fix deadlock in parallel for test
The deadlock occurs when another goroutine requests GC
during the test. When wait=true the test expects physical parallelism,
that is, that P goroutines are all active at the same time.
If GC is requested, then part of the goroutines are not scheduled,
so other goroutines deadlock.
With wait=false, goroutines finish parallel for w/o waiting for all
other goroutines.
Fixes issue 3954.

R=golang-dev, bradfitz
CC=golang-dev
http://codereview.appspot.com/6820098