Repository navigation
Define an alternate signal handler stack for HHVM threads - #9832
mszabo-wikia wants to merge 1 commit into
Conversation
|
This pull request has been imported. If you are a Meta employee, you can view this in D115599844. (Because this pull request was imported automatically, there will not be any future comments.) |
| // 128KiB stack space should be plenty. | ||
| // See `kSmallSigAltStackSize` in folly/src/folly/debugging/symbolizer/SignalHandler.cpp. | ||
| long minSigStackSize = std::max(0l, long(MINSIGSTKSZ)); | ||
| m_size = std::max(size_t(minSigStackSize), size_t(128*1024)); |
There was a problem hiding this comment.
128KiB is quite a lot of memory when multiplied by the number of threads. Can this just use SIGSTKSZ (which my machine reports to be 13504)?
folly::SignalHandler specifically checks against kSmallSigAltStackSize to decide which implementation to use. It has an implementation that uses less stack.
There was a problem hiding this comment.
Thanks! It seems I'll need to revisit this anyways because it seems that this logic segfaults anyways on September HHVM, so I might need to change a few things
There was a problem hiding this comment.
OK this actually is working fine after re-testing, sorry for the confusion. Made the change!
Today, if an HHVM thread overflows the stack, we try to run the crash
reporter and immediately crash due to having overflown the aforementioned stack.
It's possible to run into this error state even from userland code, e.g.:
```hack
async function bar(): Awaitable<void> {
$s = new \HH\Lib\Async\Semaphore(10, async (bool $should_wait) ==> {
if ($should_wait) {
await \HH\Asio\usleep(1000 * 100000);
} else {
await \HH\Asio\later();
}
});
concurrent {
await $s->waitForAsync(true);
await async {
for ($i = 0; $i < 1000000; $i++) {
await $s->waitForAsync(false);
}
};
}
}
<<__EntryPoint>>
function mymain(): void {
\HH\Asio\join(bar());
}
```
This isn't ideal because it prevents reporting tools that process
stacktrace files emitted by the crash reporter from accurately reporting
these events.
So, use `sigaltstack` to define an alternate signal handler stack for HHVM threads.
4cc3411 to
db8169a
Compare
|
@mszabo-wikia has updated the pull request. You must reimport the pull request before landing. |
Today, if an HHVM thread overflows the stack, we try to run the crash reporter and immediately crash due to having overflown the aforementioned stack. It's possible to run into this error state even from userland code, e.g.:
This isn't ideal because it prevents reporting tools that process stacktrace files emitted by the crash reporter from accurately reporting these events.
So, use
sigaltstackto define an alternate signal handler stack for HHVM threads. The folly::Symbolizer docs suggest anything below 64KiB would be insufficient, so size it 128KiB to be safe.