code review 54160043: runtime: if "panic during panic"'s stacktrace fails, do...

runtime: if "panic during panic"'s stacktrace fails, don't recurse.

[khr, 2014-01-18 21:48:27]

Hello golang-codereviews@googlegroups.com,

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

[iant, 2014-01-19 05:11:43]

https://codereview.appspot.com/54160043/diff/40001/src/pkg/runtime/panic.c
File src/pkg/runtime/panic.c (right):

https://codereview.appspot.com/54160043/diff/40001/src/pkg/runtime/panic.c#ne...
src/pkg/runtime/panic.c:374: runtime·printf("panic during panic\n");
Seems to me you need to increment m->dying here.  I don't see how the increment
down below will ever set m->dying to anything other than 1.

[khr1, 2014-01-19 07:25:58]

Good point, fixed.  PTAL.



On Sat, Jan 18, 2014 at 9:11 PM, <iant@golang.org> wrote:

>
> https://codereview.appspot.com/54160043/diff/40001/src/pkg/runtime/panic.c
> File src/pkg/runtime/panic.c (right):
>
> https://codereview.appspot.com/54160043/diff/40001/src/
> pkg/runtime/panic.c#newcode374
> src/pkg/runtime/panic.c:374: runtime·printf("panic during panic\n");
> Seems to me you need to increment m->dying here.  I don't see how the
> increment down below will ever set m->dying to anything other than 1.
>
> https://codereview.appspot.com/54160043/
>

[dvyukov, 2014-01-19 07:43:13]

On 2014/01/19 07:25:58, khr1 wrote:
You seem forgot to upload, it still contains "m->dying = 1;"

Please test it by inserting throw and nil deref into traceback.



> Good point, fixed.  PTAL.
> 
> 
> 
> On Sat, Jan 18, 2014 at 9:11 PM, <mailto:iant@golang.org> wrote:
> 
> >
> > https://codereview.appspot.com/54160043/diff/40001/src/pkg/runtime/panic.c
> > File src/pkg/runtime/panic.c (right):
> >
> > https://codereview.appspot.com/54160043/diff/40001/src/
> > pkg/runtime/panic.c#newcode374
> > src/pkg/runtime/panic.c:374: runtime·printf("panic during panic\n");
> > Seems to me you need to increment m->dying here.  I don't see how the
> > increment down below will ever set m->dying to anything other than 1.
> >
> > https://codereview.appspot.com/54160043/
> >

[iant, 2014-01-20 19:24:22]

On Sat, Jan 18, 2014 at 11:43 PM,  <dvyukov@google.com> wrote:
> On 2014/01/19 07:25:58, khr1 wrote:
> You seem forgot to upload, it still contains "m->dying = 1;"

I'm seeing chunk mismatch now....

Ian

[khr, 2014-01-21 21:52:41]

On 2014/01/20 19:24:22, iant wrote:
> On Sat, Jan 18, 2014 at 11:43 PM,  <mailto:dvyukov@google.com> wrote:
> > On 2014/01/19 07:25:58, khr1 wrote:
> > You seem forgot to upload, it still contains "m->dying = 1;"
> 
> I'm seeing chunk mismatch now....
> 
> Ian

Try again...

[iant, 2014-01-21 22:13:31]

LGTM

[khr, 2014-01-21 22:34:39]

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

runtime: if "panic during panic"'s stacktrace fails, don't recurse.

R=golang-codereviews, iant, khr, dvyukov
CC=golang-codereviews
https://codereview.appspot.com/54160043