code review 72630043: sync: give finalizers more time in TestPoolGC

sync: give finalizers more time in TestPoolGC

If we report a leak, make sure we've waited long enough to be sure.
The new sleep regimen waits 1.05 seconds before failing; the old
one waited 0.005 seconds.

(The single linux/amd64 failure in this test feels more like a
timing problem than a leak. I don't want to spend time on it unless
we're sure.)

[rsc, 2014-03-07 20:07:48]

Hello bradfitz (cc: golang-codereviews@googlegroups.com),

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

[bradfitz, 2014-03-07 20:41:28]

LGTM

But I think Dmitry wrote that.  And I thought runtime.GC was aggressive
enough now.

Worth a shot.



On Fri, Mar 7, 2014 at 12:07 PM, <rsc@golang.org> wrote:

> Reviewers: bradfitz,
>
> Message:
> Hello bradfitz (cc: golang-codereviews@googlegroups.com),
>
> I'd like you to review this change to
> https://code.google.com/p/go/
>
>
> Description:
> sync: give finalizers more time in TestPoolGC
>
> If we report a leak, make sure we've waited long enough to be sure.
> The new sleep regimen waits 1.05 seconds before failing; the old
> one waited 0.005 seconds.
>
> (The single linux/amd64 failure in this test feels more like a
> timing problem than a leak. I don't want to spend time on it unless
> we're sure.)
>
> Please review this at https://codereview.appspot.com/72630043/
>
> Affected files (+1, -1 lines):
>   M src/pkg/sync/pool_test.go
>
>
> Index: src/pkg/sync/pool_test.go
> ===================================================================
> --- a/src/pkg/sync/pool_test.go
> +++ b/src/pkg/sync/pool_test.go
> @@ -87,7 +87,7 @@
>         }
>         for i := 0; i < 5; i++ {
>                 runtime.GC()
> -               time.Sleep(time.Millisecond)
> +               time.Sleep(time.Duration(i*100+10) * time.Millisecond)
>                 // 1 pointer can remain on stack or elsewhere
>                 if atomic.LoadUint32(&fin) >= N-1 {
>                         return
>
>
>

[rsc, 2014-03-07 21:07:57]

runtime.GC is aggressive enough - they will be on the finalizer queue when
it returns. But they need time for the finalizer to run. The 85/100 on the
linux/amd64 felt to me like it just had barely not enough time. Builders
can be very slow and 5ms real time is not very much. Or it could be that 15
is a magic number. We'll see.

[rsc, 2014-03-07 21:08:19]

*** Submitted as https://code.google.com/p/go/source/detail?r=26aa53304a48 ***

sync: give finalizers more time in TestPoolGC

If we report a leak, make sure we've waited long enough to be sure.
The new sleep regimen waits 1.05 seconds before failing; the old
one waited 0.005 seconds.

(The single linux/amd64 failure in this test feels more like a
timing problem than a leak. I don't want to spend time on it unless
we're sure.)

LGTM=bradfitz
R=bradfitz
CC=golang-codereviews
https://codereview.appspot.com/72630043