code review 9303046: runtime: detect deadlocks in programs using cgo

runtime: detect deadlocks in programs using cgo
When cgo is used, runtime creates an additional M to handle callbacks on threads not created by Go.
This effectively disabled deadlock detection, which is a right thing, because Go program can be blocked
and only serve callbacks on external threads.
This also disables deadlock detection under race detector, because it happens to use cgo.
With this change the additional M is created lazily on first cgo call. So deadlock detector
works for programs that import "C", "net" or "net/http/pprof" but do not use them in fact.
Also fixes deadlock detector under race detector.
It should be fine to create the M later, because C code can not call into Go before first cgo call,
because C code does not know when Go initialization has completed. So a Go program need to call into C
first either to create an external thread, or notify a thread created in global ctor that Go
initialization has completed.
Fixes issue 4973.
Fixes issue 5475.

[dvyukov, 2013-05-15 11:57:55]

Hello golang-dev@googlegroups.com,

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

[dvyukov, 2013-05-15 12:27:42]

PTAL
Added diagnostics for early cgo callbacks

[minux1, 2013-05-15 12:32:50]

On Wed, May 15, 2013 at 7:57 PM, <dvyukov@google.com> wrote:

> Description:
> runtime: detect deadlocks in programs using cgo
> When cgo is used, runtime creates an additional M to handle callbacks on
> threads not created by Go.
> This effectively disabled deadlock detection, which is a right thing,
> because Go program can be blocked
> and only serve callbacks on external threads.
> This also disables deadlock detection under race detector, because it
> happens to use cgo.
> With this change the additional M is created lazily on first cgo call.
> So deadlock detector
> works for programs that import "C", "net" or "net/http/pprof" but do not
> use them in fact.
> Also fixes deadlock detector under race detector.
> It should be fine to create the M later, because C code can not call
> into Go before first cgo call,
> because C code does not know when Go initialization has completed. So a
> Go program need to call into C
> first either to create an external thread, or notify a thread created in
> global ctor that Go
> initialization has completed.
>
Thank you for the detailed explanation.

what if Go code and foreign thread are using polling and atomic
instructions to signify
the completion of Go initialization?

I think a better approach would be to create the extra M only when there is
exported Go
functions in this program, the linker could provide such information.

[dvyukov, 2013-05-15 12:40:41]

On Wed, May 15, 2013 at 4:32 PM, minux <minux.ma@gmail.com> wrote:
>
> On Wed, May 15, 2013 at 7:57 PM, <dvyukov@google.com> wrote:
>>
>> Description:
>> runtime: detect deadlocks in programs using cgo
>> When cgo is used, runtime creates an additional M to handle callbacks on
>> threads not created by Go.
>> This effectively disabled deadlock detection, which is a right thing,
>> because Go program can be blocked
>> and only serve callbacks on external threads.
>> This also disables deadlock detection under race detector, because it
>> happens to use cgo.
>> With this change the additional M is created lazily on first cgo call.
>> So deadlock detector
>> works for programs that import "C", "net" or "net/http/pprof" but do not
>> use them in fact.
>> Also fixes deadlock detector under race detector.
>> It should be fine to create the M later, because C code can not call
>> into Go before first cgo call,
>> because C code does not know when Go initialization has completed. So a
>> Go program need to call into C
>> first either to create an external thread, or notify a thread created in
>> global ctor that Go
>> initialization has completed.
>
> Thank you for the detailed explanation.
>
> what if Go code and foreign thread are using polling and atomic instructions
> to signify
> the completion of Go initialization?

Is it possible to link Go var and C var together?
I think gcc will have a problem with . in symbol name.

Of course there can be other weird cases, e.g. use filesystem of
network to signal initialization. But I think we can ignore this,
especially since the fix is simple -- make a fake cgo call.

Also note that this can only happen if C creates the thread in a global ctor.

> I think a better approach would be to create the extra M only when there is
> exported Go
> functions in this program, the linker could provide such information.

Won't work for race detector, because it does contain callbacks.

[minux1, 2013-05-15 19:04:27]

On Wed, May 15, 2013 at 8:40 PM, Dmitry Vyukov <dvyukov@google.com> wrote:

> Is it possible to link Go var and C var together?
> I think gcc will have a problem with . in symbol name.
>
one can use __asm__("pkg.A") to make a symbol name anything he wants,
however, this is very cumbersome to do (my example needs one Go package
and one main package with global constructors in C).

>
> Of course there can be other weird cases, e.g. use filesystem of
> network to signal initialization. But I think we can ignore this,
> especially since the fix is simple -- make a fake cgo call.
>
right.

>
> Also note that this can only happen if C creates the thread in a global
> ctor.
>
> > I think a better approach would be to create the extra M only when there
> is
> > exported Go
> > functions in this program, the linker could provide such information.
>
> Won't work for race detector, because it does contain callbacks.
>
but race detector won't use the callback from arbitrary C thread, right?
we can special case race detector here (we've been already specially
handling cgo calls issued by the race detector). in fact, iiuc, race
detector
won't do callback from foreign C thread so its callback actually doesn't
need
the extram, we can just treat runtime/race as not containing exported Go
func.

the reason why i propose that approach is that i think being able to use
the deadlock detector when using package net (and os/user) is important.

[dvyukov, 2013-05-15 19:12:57]

On Wed, May 15, 2013 at 11:04 PM, minux <minux.ma@gmail.com> wrote:
>
> On Wed, May 15, 2013 at 8:40 PM, Dmitry Vyukov <dvyukov@google.com> wrote:
>>
>> Is it possible to link Go var and C var together?
>> I think gcc will have a problem with . in symbol name.
>
> one can use __asm__("pkg.A") to make a symbol name anything he wants,
> however, this is very cumbersome to do (my example needs one Go package
> and one main package with global constructors in C).
>>
>>
>> Of course there can be other weird cases, e.g. use filesystem of
>> network to signal initialization. But I think we can ignore this,
>> especially since the fix is simple -- make a fake cgo call.
>
> right.
>>
>>
>> Also note that this can only happen if C creates the thread in a global
>> ctor.
>>
>> > I think a better approach would be to create the extra M only when there
>> > is
>> > exported Go
>> > functions in this program, the linker could provide such information.
>>
>> Won't work for race detector, because it does contain callbacks.
>
> but race detector won't use the callback from arbitrary C thread, right?
> we can special case race detector here (we've been already specially
> handling cgo calls issued by the race detector). in fact, iiuc, race
> detector
> won't do callback from foreign C thread so its callback actually doesn't
> need
> the extram, we can just treat runtime/race as not containing exported Go
> func.
>
> the reason why i propose that approach is that i think being able to use
> the deadlock detector when using package net (and os/user) is important.


A single timer, syscall or outstanding network read disables the
deadlock detector. So I do not think current implementation is useful
for anything other than toy programs. In fact, current network poller
implementation (as well as previous) disabled deadlock detector even
w/o outstanding reads. So it does not work with net.

[iant, 2013-05-22 18:51:23]

LGTM

[dvyukov, 2013-05-22 18:59:53]

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

runtime: detect deadlocks in programs using cgo
When cgo is used, runtime creates an additional M to handle callbacks on threads
not created by Go.
This effectively disabled deadlock detection, which is a right thing, because Go
program can be blocked
and only serve callbacks on external threads.
This also disables deadlock detection under race detector, because it happens to
use cgo.
With this change the additional M is created lazily on first cgo call. So
deadlock detector
works for programs that import "C", "net" or "net/http/pprof" but do not use
them in fact.
Also fixes deadlock detector under race detector.
It should be fine to create the M later, because C code can not call into Go
before first cgo call,
because C code does not know when Go initialization has completed. So a Go
program need to call into C
first either to create an external thread, or notify a thread created in global
ctor that Go
initialization has completed.
Fixes issue 4973.
Fixes issue 5475.

R=golang-dev, minux.ma, iant
CC=golang-dev
https://codereview.appspot.com/9303046