• Austin Clements's avatar
    runtime: never allocate during an unrecoverable panic · 2edc4d46
    Austin Clements authored
    Currently, startpanic_m (which prepares for an unrecoverable panic)
    goes out of its way to make it possible to allocate during panic
    handling by allocating an mcache if there isn't one.
    
    However, this is both potentially dangerous and unnecessary.
    Allocating an mcache is a generally complex thing to do in an already
    precarious situation. Specifically, it requires obtaining the heap
    lock, and there's evidence that this may be able to deadlock (#23360).
    However, it's also unnecessary because we never allocate from the
    unrecoverable panic path.
    
    This didn't use to be the case. The call to allocmcache was introduced
    long ago, in CL 7388043, where it was in preparation for separating Ms
    and Ps and potentially running an M without an mcache. At the time,
    after calling startpanic, the runtime could call String and Error
    methods on panicked values, which could do anything including
    allocating. That was generally unsafe even at the time, and CL 19792
    fixed this be pre-printing panic messages before calling startpanic.
    As a result, we now no longer allocate after calling startpanic.
    
    This CL not only removes the allocmcache call, but goes a step further
    to explicitly disallow any allocation during unrecoverable panic
    handling, even in situations where it might be safe. This way, if
    panic handling ever does an allocation that would be unsafe in unusual
    circumstances, we'll know even if it happens during normal
    circumstances.
    
    This would help with debugging #23360, since the deadlock in
    allocmcache is currently masking the real failure.
    
    Beyond all.bash, I manually tested this change by adding panics at
    various points in early runtime init, signal handling, and the
    scheduler to check unusual panic situations.
    
    Change-Id: I85df21e2b4b20c6faf1f13fae266c9339eebc061
    Reviewed-on: https://go-review.googlesource.com/88835
    Run-TryBot: Austin Clements <austin@google.com>
    TryBot-Result: Gobot Gobot <gobot@golang.org>
    Reviewed-by: 's avatarKeith Randall <khr@golang.org>
    Reviewed-by: 's avatarIan Lance Taylor <iant@golang.org>
    2edc4d46
panic.go 20.2 KB