code review 5844051: runtime: restore deadlock detection in the simplest case.

runtime: restore deadlock detection in the simplest case.

Fixes issue 3342.

[remyoudompheng, 2012-03-17 11:20:57]

Hello golang-dev@googlegroups.com (cc: golang-dev@googlegroups.com,
remy@archlinux.org),

I'd like you to review this change to
https://go.googlecode.com/hg/

[remyoudompheng, 2012-03-17 11:21:25]

I'd like someone to check I am not writing garbage here.

Rémy.

[iant, 2012-03-18 04:36:51]

LGTM perhaps with the mentioned change.

http://codereview.appspot.com/5844051/diff/4001/src/pkg/runtime/mheap.c
File src/pkg/runtime/mheap.c (right):

http://codereview.appspot.com/5844051/diff/4001/src/pkg/runtime/mheap.c#newco...
src/pkg/runtime/mheap.c:366: 
I think the place to call this is immediately before calling entersyscall, not
immediately after calling exitsyscall.  I think it should work either way, but
calling gosched before sleeping will avoid a pointless hiccup after the
scavenger wakes up.

[remyoudompheng, 2012-03-20 19:42:38]

Hello golang-dev@googlegroups.com, iant@golang.org (cc:
golang-dev@googlegroups.com, remy@archlinux.org),

Please take another look.

[remyoudompheng, 2012-03-20 19:43:10]

http://codereview.appspot.com/5844051/diff/4001/src/pkg/runtime/mheap.c
File src/pkg/runtime/mheap.c (right):

http://codereview.appspot.com/5844051/diff/4001/src/pkg/runtime/mheap.c#newco...
src/pkg/runtime/mheap.c:366: 
On 2012/03/18 04:36:51, iant wrote:
> I think the place to call this is immediately before calling entersyscall, not
> immediately after calling exitsyscall.  I think it should work either way, but
> calling gosched before sleeping will avoid a pointless hiccup after the
> scavenger wakes up.

Done.

[iant, 2012-03-20 20:18:57]

LGTM and fixes an issue but may be too risky at this point.  I would vote for
putting it in but leaving for r for final decision.

[remyoudompheng, 2012-03-20 20:20:03]

It's fine to me if it's deferred. According to my tests the problem only shows
up if you have a single goroutine, in which case the deadlock should be
trivially visible.

[dave_cheney.net, 2012-03-20 20:40:34]

Remé, thank you for this CL. I'm concerned that it is too risky this close to
Go1 so I think it should be left til after the release. 

On 21/03/2012, at 7:18, iant@golang.org wrote:

> LGTM and fixes an issue but may be too risky at this point.  I would
> vote for putting it in but leaving for r for final decision.
> 
> http://codereview.appspot.com/5844051/

[r, 2012-03-21 01:32:44]

I'd like to give rsc a chance to look this over before checking it in. The
problem it solves is not important enough to offset the risk that comes with
changing such fundamental code. I vote for holding until after we cut Go 1 -
soon.

[rsc, 2012-03-26 14:31:34]

Let's wait until after Go 1.  I don't fully understand this.

[rsc, 2012-03-27 03:06:35]

*** Submitted as 5d0322034aa8 ***

runtime: restore deadlock detection in the simplest case.

Fixes issue 3342.

R=iant, r, dave, rsc
CC=golang-dev, remy
http://codereview.appspot.com/5844051

Committer: Russ Cox <rsc@golang.org>