Skip to content

Commit

Permalink
FIX Prevent segfaults in CLI runner
Browse files Browse the repository at this point in the history
After many hours of debugging, I minimized the problem down to this:
emscripten-core/emscripten#22052

Thanks to @henryiii for reporting. See thread here for my comments while I was
debugging:
scikit-hep/boost-histogram#938
  • Loading branch information
hoodmane committed Jun 4, 2024
1 parent 7cf233e commit d6c42f6
Show file tree
Hide file tree
Showing 3 changed files with 12 additions and 9 deletions.
8 changes: 4 additions & 4 deletions src/core/error_handling.c
Original file line number Diff line number Diff line change
Expand Up @@ -119,13 +119,13 @@ EM_JS(JsVal, restore_stderr, (void), {
/**
* Wrap the exception in a JavaScript PythonError object.
*
* The return value of this function is always a valid hiwire ID to an error
* object. It never returns NULL.
* The return value of this function is always a JavaScript error object. It
* never returns null.
*
* We are cautious about leaking the Python stack frame, so we don't increment
* the reference count on the exception object, we just store a pointer to it.
* Later we can check if this pointer is equal to sys.last_exc and if so
* restore the exception (see restore_sys_last_exception).
* Later we can check if this pointer is equal to sys.last_exc and if so restore
* the exception (see restore_sys_last_exception).
*
* WARNING: dereferencing the error pointer stored on the PythonError is a
* use-after-free bug.
Expand Down
3 changes: 3 additions & 0 deletions src/js/load-package.ts
Original file line number Diff line number Diff line change
Expand Up @@ -555,6 +555,9 @@ export async function loadPackage(
Array.from(toLoad.values()).map(({ installPromise }) => installPromise),
);

// Warning: this sounds like it might not do anything important, but it
// fills in the GOT. There can be segfaults if we leave it out.
// See https://github.com/emscripten-core/emscripten/issues/22052
Module.reportUndefinedSymbols();
if (loadedPackageData.size > 0) {
const successNames = Array.from(loadedPackageData, (pkg) => pkg.name)
Expand Down
10 changes: 5 additions & 5 deletions src/templates/python
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,10 @@ async function main() {
console.error(e);
}
}
// Warning: this sounds like it might not do anything important, but it
// fills in the GOT. There can be segfaults if we leave it out.
// See https://github.com/emscripten-core/emscripten/issues/22052
py._module.reportUndefinedSymbols();
py.runPython(
`
Expand Down Expand Up @@ -176,14 +180,10 @@ async function main() {
try {
errcode = py._module._run_main();
} catch (e) {
// If someone called exit, just exit with the right return code.
if(e.constructor.name === "ExitStatus"){
process.exit(e.status);
}
// Otherwise if there is some sort of error, include the Python
// tracebook in addition to the JavaScript traceback
py._module._dump_traceback();
throw e;
py._api.fatal_error(e);
}
if (errcode) {
process.exit(errcode);
Expand Down

0 comments on commit d6c42f6

Please sign in to comment.