code review 7324058: runtime/cgo: fix deadlock involving signals on darwin

runtime/cgo: fix deadlock involving signals on darwin
sigprocmask() is process-wide on darwin, so two concurrent
libcgo_sys_thread_start() can result in all signals permanently
blocked, which in particular blocks handling of nil derefs.
Fixes issue 4833.

[dvyukov, 2013-02-20 08:31:23]

Hello golang-dev@googlegroups.com,

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

[dave_cheney.net, 2013-02-20 08:38:23]

On 2013/02/20 08:31:23, dvyukov wrote:
> Hello mailto:golang-dev@googlegroups.com,
> 
> I'd like you to review this change to
> https://dvyukov%2540google.com%40code.google.com/p/go/

Nice catch, I can test on Snow Leopard / 386 and Lion / amd64 when I get home

[dvyukov, 2013-02-20 08:40:35]

On 2013/02/20 08:38:23, dfc wrote:
> On 2013/02/20 08:31:23, dvyukov wrote:
> > Hello mailto:golang-dev@googlegroups.com,
> > 
> > I'd like you to review this change to
> > https://dvyukov%252540google.com%2540code.google.com/p/go/
> 
> Nice catch, I can test on Snow Leopard / 386 and Lion / amd64 when I get home

I failed to reproduce it with current runtime, because threads are created under
a big mutex. It deadlocks with my scheduler patch, though.

[dave_cheney.net, 2013-02-20 08:42:03]

Thanks. I'll make sure I patch my runtime.

On Wed, Feb 20, 2013 at 7:40 PM,  <dvyukov@google.com> wrote:
> On 2013/02/20 08:38:23, dfc wrote:
>>
>> On 2013/02/20 08:31:23, dvyukov wrote:
>> > Hello mailto:golang-dev@googlegroups.com,
>> >
>> > I'd like you to review this change to
>> > https://dvyukov%252540google.com%2540code.google.com/p/go/
>
>
>> Nice catch, I can test on Snow Leopard / 386 and Lion / amd64 when I
>
> get home
>
> I failed to reproduce it with current runtime, because threads are
> created under a big mutex. It deadlocks with my scheduler patch, though.
>
>
> https://codereview.appspot.com/7324058/

[dvyukov, 2013-02-21 05:52:41]

ping

[dave_cheney.net, 2013-02-21 05:53:43]

On 2013/02/21 05:52:41, dvyukov wrote:
> ping

Can you please point me to the latest uncommitted CL's for the scheduler that I
need to apply so I can test this CL.

[dvyukov, 2013-02-21 06:21:15]

Just merged Russ' changes.
Try hg clpatch 7314062 now.

On Thu, Feb 21, 2013 at 9:53 AM,  <dave@cheney.net> wrote:
> On 2013/02/21 05:52:41, dvyukov wrote:
>>
>> ping
>
>
> Can you please point me to the latest uncommitted CL's for the scheduler
> that I need to apply so I can test this CL.
>
> https://codereview.appspot.com/7324058/

[dave_cheney.net, 2013-02-22 01:00:46]

Hi,

I'm still chasing merge conflicts on 7314062, then I can test 7324058

[dvyukov, 2013-02-22 04:14:07]

The things are changing too fast...
Perhaps, you may update to a revision w/o conflicts (the one suggested
by hg clpatch), and apply the patches to it.


On Fri, Feb 22, 2013 at 5:00 AM,  <dave@cheney.net> wrote:
> Hi,
>
> I'm still chasing merge conflicts on 7314062, then I can test 7324058
>
> https://codereview.appspot.com/7324058/

[dave_cheney.net, 2013-02-22 10:45:32]

https://codereview.appspot.com/7379046

On Fri, Feb 22, 2013 at 3:14 PM, Dmitry Vyukov <dvyukov@google.com> wrote:
> The things are changing too fast...
> Perhaps, you may update to a revision w/o conflicts (the one suggested
> by hg clpatch), and apply the patches to it.
>
>
> On Fri, Feb 22, 2013 at 5:00 AM,  <dave@cheney.net> wrote:
>> Hi,
>>
>> I'm still chasing merge conflicts on 7314062, then I can test 7324058
>>
>> https://codereview.appspot.com/7324058/

[dave_cheney.net, 2013-02-22 10:46:06]

Ooops, please ignore my last message, wrong CL.

[rsc, 2013-02-25 21:36:11]

LGTM

[rsc, 2013-02-25 21:36:33]

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

runtime/cgo: fix deadlock involving signals on darwin
sigprocmask() is process-wide on darwin, so two concurrent
libcgo_sys_thread_start() can result in all signals permanently
blocked, which in particular blocks handling of nil derefs.
Fixes issue 4833.

R=golang-dev, dave, rsc
CC=golang-dev
https://codereview.appspot.com/7324058

Committer: Russ Cox <rsc@golang.org>

[minux1, 2013-02-26 16:39:12]

Hi dvyukov, just a note about placement of cgo-dependent tests.
please place them into crash_cgo_test.go instead of crash_test.go,
or the test will run on cgo disabled hosts (e.g. FreeBSD/arm) and
cause test failures (
http://build.golang.org/log/152248fec98bb3c45d89fd9e01b57724d06f92ea)
Thank you.

I will fix the issue in CL 7393063.