[go-nuts] Divide by zero in tls?

[Daniel Smith]

Hi, my web app just went down with the below stack trace.

Two questions:

1. Has this been reported (a search of the mailing list didn't show me anything)? It *may* be surious as I'm not totally certain I built with a stable go release. (I thought I built against the first "stable" release, but there's some chance I built against a random dev revision a bit after that...)

2. As you can see, the goroutine that crashed originated in the http package and never got to my own code. Is there any way to catch such a panic so it doesn't take my program down in flames? This seems like an important question whether or not this particular crash is due to my own ineptness with hg. :)

Thanks!




panic: runtime error: integer divide by zero

[signal 0x8 code=0x1 addr=0x56b9dc pc=0x56b9dc]

runtime.panic+0xac /home/daniel/go/src/pkg/runtime/proc.c:1034
    runtime.panic(0x638020, 0xf8432f7df0)
----- stack segment boundary -----
runtime.panicstring+0xa3 /home/daniel/go/src/pkg/runtime/runtime.c:116
    runtime.panicstring(0x7eed24, 0x560176)
runtime.sigpanic+0x134 /home/daniel/go/src/pkg/runtime/linux/thread.c:298
    runtime.sigpanic()
big.divWW+0xa /home/daniel/go/src/pkg/big/arith_amd64.s:59
    big.divWW(0xc38d50e195fe2dd1, 0x652ab5d2c5b4dc0a, 0x1ff, 0x130000000f, 0xf8441dd9b0, ...)
big.nat·divLarge+0x5a6 /home/daniel/go/src/pkg/big/nat.go:558
    big.nat·divLarge(0x0, 0x0, 0xf844334420, 0x1600000012, 0xf844334370, ...)
big.nat·div+0x389 /home/daniel/go/src/pkg/big/nat.go:517
    big.nat·div(0x0, 0x0, 0xf844334370, 0x1600000011, 0xf844334370, ...)
big.*Int·QuoRem+0x69 /home/daniel/go/src/pkg/big/int.go:207
    big.*Int·QuoRem(0xf8443321c0, 0xf8443321a0, 0xf8442ed1e0, 0xf8443321a0, 0x2600000000, ...)
big.*Int·Mod+0x98 /home/daniel/go/src/pkg/big/int.go:240
    big.*Int·Mod(0xf8443321a0, 0xf8443321a0, 0xf8442ed1e0, 0xf8443321a0, 0xf844332020, ...)
crypto/elliptic.*Curve·doubleJacobian+0xbe /home/daniel/go/src/pkg/crypto/elliptic/elliptic.go:150
    crypto/elliptic.*Curve·doubleJacobian(0xf8442e2660, 0xf8443320a0, 0xf8443320e0, 0xf844332120, 0xf8441d9da0, ...)
crypto/elliptic.*Curve·ScalarMult+0x13e /home/daniel/go/src/pkg/crypto/elliptic/elliptic.go:222
    crypto/elliptic.*Curve·ScalarMult(0xf8442e2660, 0xf8442ed240, 0xf8442ed260, 0xf843fcd5a0, 0x4200000042, ...)
crypto/elliptic.*Curve·ScalarBaseMult+0x52 /home/daniel/go/src/pkg/crypto/elliptic/elliptic.go:245
    crypto/elliptic.*Curve·ScalarBaseMult(0xf8442e2660, 0xf843fcd5a0, 0x4200000042, 0x4200000042, 0x4200000042, ...)
crypto/elliptic.*Curve·GenerateKey+0x1a9 /home/daniel/go/src/pkg/crypto/elliptic/elliptic.go:267
    crypto/elliptic.*Curve·GenerateKey(0xf8442e2660, 0xf840001cf0, 0xf840001cc0, 0xf843fcd5a0, 0x4200000042, ...)
crypto/tls.*ecdheRSAKeyAgreement·generateServerKeyExchange+0x11e /home/daniel/go/src/pkg/crypto/tls/key_agreement.go:132
    crypto/tls.*ecdheRSAKeyAgreement·generateServerKeyExchange(0xf841a0b8a0, 0xf8400617e0, 0xf841ead780, 0xf84148cde0, 0xf800001383, ...)
crypto/tls.*Conn·serverHandshake+0x957 /home/daniel/go/src/pkg/crypto/tls/handshake_server.go:121
    crypto/tls.*Conn·serverHandshake(0xf84459d700, 0x0, 0x0, 0x0)
crypto/tls.*Conn·Handshake+0xfd /home/daniel/go/src/pkg/crypto/tls/conn.go:753
    crypto/tls.*Conn·Handshake(0xf84459d700, 0x0, 0x0, 0x4556c2)
crypto/tls.*Conn·Read+0x3d /home/daniel/go/src/pkg/crypto/tls/conn.go:705
    crypto/tls.*Conn·Read(0xf84459d700, 0xf841490000, 0x100000001000, 0x0, 0x0, ...)
bufio.*Reader·fill+0x154 /home/daniel/go/src/pkg/bufio/bufio.go:98
    bufio.*Reader·fill(0xf8445e91c0, 0x100000000000)
bufio.*Reader·ReadSlice+0x287 /home/daniel/go/src/pkg/bufio/bufio.go:267
    bufio.*Reader·ReadSlice(0xf8445e91c0, 0xa, 0x0, 0x0, 0x0, ...)
bufio.*Reader·ReadBytes+0xdd /home/daniel/go/src/pkg/bufio/bufio.go:327
    bufio.*Reader·ReadBytes(0xf8445e91c0, 0x44fc0a, 0x0, 0x0, 0x0, ...)
net/textproto.*Reader·ReadLineBytes+0x3b /home/daniel/go/src/pkg/net/textproto/reader.go:43
    net/textproto.*Reader·ReadLineBytes(0xf8432f7d70, 0x4504c2, 0xe0, 0x100000000)
net/textproto.*Reader·ReadLine+0x25 /home/daniel/go/src/pkg/net/textproto/reader.go:36
    net/textproto.*Reader·ReadLine(0xf8432f7d70, 0xf8449b6c40, 0xffffffff, 0x0)
http.ReadRequest+0x7c /home/daniel/go/src/pkg/http/request.go:431
    http.ReadRequest(0xf8445e91c0, 0xf8449b6c40, 0x0, 0x0, 0xf84408f538, ...)
http.*conn·readRequest+0x73 /home/daniel/go/src/pkg/http/server.go:171
    http.*conn·readRequest(0xf8445bdd20, 0xf842b7b440, 0x0, 0x0, 0xf843ea9a00, ...)
http.*conn·serve+0x25 /home/daniel/go/src/pkg/http/server.go:439
    http.*conn·serve(0xf8445bdd20, 0x0)
runtime.goexit /home/daniel/go/src/pkg/runtime/proc.c:178
    runtime.goexit()
----- goroutine created by -----
http.*Server·Serve+0x203 /home/daniel/go/src/pkg/http/server.go:801


--
Daniel Smith
http://dailyjoseki.com/
http://www.schaumburggoclub.org/

[Russ Cox]

> 2. As you can see, the goroutine that crashed originated in the http package
> and never got to my own code. Is there any way to catch such a panic so it
> doesn't take my program down in flames? This seems like an important
> question whether or not this particular crash is due to my own ineptness
> with hg. :)

No, there isn't. The http package would have to do the recovering.

> 1. Has this been reported (a search of the mailing list didn't show me
> anything)? It *may* be surious as I'm not totally certain I built with a
> stable go release. (I thought I built against the first "stable" release,
> but there's some chance I built against a random dev revision a bit after
> that...)

> panic: runtime error: integer divide by zero

>
> [signal 0x8 code=0x1 addr=0x56b9dc pc=0x56b9dc]
>
> runtime.panic+0xac /home/daniel/go/src/pkg/runtime/proc.c:1034
>     runtime.panic(0x638020, 0xf8432f7df0)
> ----- stack segment boundary -----
> runtime.panicstring+0xa3 /home/daniel/go/src/pkg/runtime/runtime.c:116
>     runtime.panicstring(0x7eed24, 0x560176)
> runtime.sigpanic+0x134 /home/daniel/go/src/pkg/runtime/linux/thread.c:298
>     runtime.sigpanic()
> big.divWW+0xa /home/daniel/go/src/pkg/big/arith_amd64.s:59
>     big.divWW(0xc38d50e195fe2dd1, 0x652ab5d2c5b4dc0a, 0x1ff, 0x130000000f,
> 0xf8441dd9b0, ...)

It looks to me like divWW was asked to divide
c38d50e195fe2dd1652ab5d2c5b4dc0a / 1ff,
which is not going to fit in a 64-bit quotient, so it trapped. It
looks like either memory
corruption or a bug in divLarge. Without the actual bigger numbers
involved it is hard
to say, and they are gone.

Russ

[Daniel Smith]

On Wed, Jun 1, 2011 at 9:40 PM, Russ Cox <r...@golang.org> wrote:

No, there isn't.  The http package would have to do the recovering.

That's what I thought. Maybe I'm just paranoid now, but it seems like a rather big problem that something I have no control over can crash my program and there's no way I can recover. Does anyone else think it might be worthwhile for http to catch panics that don't originate in the user's code? I can think of a few different ways of modifying http to accomplish that...

It looks to me like divWW was asked to divide
c38d50e195fe2dd1652ab5d2c5b4dc0a / 1ff,
which is not going to fit in a 64-bit quotient, so it trapped.  It
looks like either memory
corruption or a bug in divLarge.  Without the actual bigger numbers
involved it is hard
to say, and they are gone.

I don't think I use unsafe anywhere, and I don't link to any C code or anything...

[Russ Cox]

> That's what I thought. Maybe I'm just paranoid now, but it seems like a
> rather big problem that something I have no control over can crash my
> program and there's no way I can recover. Does anyone else think it might be
> worthwhile for http to catch panics that don't originate in the user's code?
> I can think of a few different ways of modifying http to accomplish that...

Probably.

Russ

[Russ Cox]

I see the bug. Thanks for the detailed report.

Russ

[Brad Fitzpatrick]

I've been debating doing that too.  I haven't quite figured out the panic policies and when I can use them.

Can I wrap each http connection's goroutine with a recover, and if it recovers in user code, serves a 500 with no details if nothing's been sent yet?  But always log details?

[Russ Cox]

> I've been debating doing that too.  I haven't quite figured out the panic
> policies and when I can use them.
> Can I wrap each http connection's goroutine with a recover, and if it
> recovers in user code, serves a 500 with no details if nothing's been sent
> yet?  But always log details?

Seems reasonable. I might just hang up.

[Daniel Smith]

On Thu, Jun 2, 2011 at 8:39 AM, Russ Cox <r...@golang.org> wrote:

> ... Does anyone else think it might be

> worthwhile for http to catch panics that don't originate in the user's code?
> I can think of a few different ways of modifying http to accomplish that...

Probably.

I'll think about it, then. Current thoughts below, in response to Brad.

I see the bug.  Thanks for the detailed report.

Awesome! I'm a big fan of the language, and fast responses on stuff like this makes it even better. :)

On Thu, Jun 2, 2011 at 9:07 AM, Brad Fitzpatrick <brad...@golang.org> wrote:

I've been debating doing that too.  I haven't quite figured out the panic policies and when I can use them.

Can I wrap each http connection's goroutine with a recover, and if it recovers in user code, serves a 500 with no details if nothing's been sent yet?  But always log details?

If you're talking about modifying the http package itself, I'd prefer one of:

1) It hangs up, like Russ says, or at least lets me pick the error message.

2) It hangs up and sends a stack trace over a user specified error channel.

3) The http.Server object lets the user specify a panic catching function, which it defers at the start of every goroutine.

The code in http has changed a bit since I last read it, but I think 3 would be fairly easy to implement and the most flexible option. Users would give it a function like this one (copied from my code):

func PanicProtectServeError(c http.ResponseWriter, req *http.Request) {
    if x := recover(); x != nil {
        fmt.Println("****Panic caught:", x)
        fmt.Println("While handling request:", req.RawURL)
        fmt.Println("\n", req.Header, "\n")
        if !HidePanics {
            panic(x)  // go back to panicking
        }
        PrintCallStack();
        fmt.Println("****Panic recovered. Returning error\n\n")
        c.WriteHeader(http.StatusInternalServerError)
        
        SendAdminMail("Panic caught (error served)!", fmt.Sprintf("panic: %s\r\v%s", x, CallStackToString()), false)
    }
}

"Never crash" seems like a reasonable requirement from programs that use the http package, so I think it'd make sense to do something like this.

[Russ Cox]

I am pretty sure the http package should just hang up.

If a particular server wants something fancy it can do
that easily enough (look at the handlers Andrew and Rob
used in their talk at Google I/O this year).

> 3) The http.Server object lets the user specify a panic catching function,
> which it defers at the start of every goroutine.
> The code in http has changed a bit since I last read it, but I think 3 would
> be fairly easy to implement and the most flexible option. Users would give
> it a function like this one (copied from my code):
>
> func PanicProtectServeError(c http.ResponseWriter, req *http.Request) {
> if x := recover(); x != nil {
> fmt.Println("****Panic caught:", x)
> fmt.Println("While handling request:", req.RawURL)
> fmt.Println("\n", req.Header, "\n")
> if !HidePanics {
> panic(x) // go back to panicking
> }
> PrintCallStack();
> fmt.Println("****Panic recovered. Returning error\n\n")
> c.WriteHeader(http.StatusInternalServerError)
>
> SendAdminMail("Panic caught (error served)!", fmt.Sprintf("panic:
> %s\r\v%s", x, CallStackToString()), false)
> }
> }

This doesn't apply to your crash; the server failed before
it even got a chance to invoke a handler.

Russ

[Daniel Smith]

On Thu, Jun 2, 2011 at 11:03 AM, Russ Cox <r...@golang.org> wrote:

I am pretty sure the http package should just hang up.

If a particular server wants something fancy it can do
that easily enough (look at the handlers Andrew and Rob
used in their talk at Google I/O this year).

I'll go check it out. 

This doesn't apply to your crash; the server failed before
it even got a chance to invoke a handler.

Well, my thinking was that any goroutine that the http server starts would defer a user setable function before it does anything else, so that it would be able to have caught this crash.

E.g., user code writes:

Server.SetPanicHandler(PanicProtectServeError)

and http's conn.serve method (for example) would start with something like

func (c *conn) serve() {

    defer c.handler.PanicHandler()

    ....

I see from looking at the code that it wouldn't have the ResponseWriter or request available at that point to pass to a hypothetical panic protection handler, so that part of it won't work. Still, as a user, I definitely want to be able to not crash but still get a stack trace.

[Russ Cox]

Options aren't the right option here.
We'll make http do the right thing.

Russ

[Brad Fitzpatrick]

Done:

http://code.google.com/p/go/source/detail?r=d27c5e1bd8e4

If you want fancier behavior, you can wrap your Handler in a PanicCatchingHandler.

But at least with this your server will stay alive by default.