code review 133470043: runtime: avoid race with forcegc helper

runtime: avoid race with forcegc helper

While we are here, give the gc helper a real function name
that will appear in stack traces.

[rsc, 2014-09-02 20:23:34]

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

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

[rlh, 2014-09-02 21:00:25]

On 2014/09/02 20:23:34, rsc wrote:
> Hello rlh (cc: mailto:golang-codereviews@googlegroups.com),
> 
> I'd like you to review this change to
> https://code.google.com/p/go/

LGTM

[rsc, 2014-09-02 21:33:39]

cc += dvyukov

[rsc, 2014-09-02 23:18:50]

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

runtime: avoid race with forcegc helper

While we are here, give the gc helper a real function name
that will appear in stack traces.

LGTM=rlh
R=rlh
CC=dvyukov, golang-codereviews
https://codereview.appspot.com/133470043

[gobot, 2014-09-03 03:05:53]

This CL appears to have broken the linux-amd64-temp builder.
See http://build.golang.org/log/25091c1ccd65a618d6eaf9594876c6309d84cff5

[dvyukov, 2014-09-03 03:44:15]

https://codereview.appspot.com/133470043/diff/40002/src/pkg/runtime/proc.c
File src/pkg/runtime/proc.c (right):

https://codereview.appspot.com/133470043/diff/40002/src/pkg/runtime/proc.c#ne...
src/pkg/runtime/proc.c:2849: if(runtime·forcegc.g != nil) {
there is no race here

this code is executed only when forcegc.idle==1; forcegc.idle is set only when
forcegc.g is already set
so this just increases level of indentation and makes reader think that
forcegc.g somehow can be nil here

[rsc, 2014-09-03 14:28:09]

Somehow Rick saw a crash where forcegc.g was nil there. I agree with you that
idle should make that impossible. If it keeps happening I will try to get more
details.

[dvyukov, 2014-09-03 14:39:08]

Such crash was possible before
https://code.google.com/p/go/source/detail?r=711e11b79b481b9a5a9d51b993832f5d...
I suspect that the crash happened in an old client.

On Wed, Sep 3, 2014 at 6:28 PM,  <rsc@golang.org> wrote:
> Somehow Rick saw a crash where forcegc.g was nil there. I agree with you
> that idle should make that impossible. If it keeps happening I will try
> to get more details.
>
>
> https://codereview.appspot.com/133470043/

[rsc, 2014-09-03 15:10:11]

On Wed, Sep 3, 2014 at 10:38 AM, Dmitry Vyukov <dvyukov@google.com> wrote:

> Such crash was possible before
>
>
https://code.google.com/p/go/source/detail?r=711e11b79b481b9a5a9d51b993832f5d...
> I suspect that the crash happened in an old client.
>

Yes, we just reached the same conclusion, and we confirmed that the crash
was showing the gchelper written in C. I will take out the change.

Russ