Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Potential Regression. Calling EncodeMeshToDracoBuffer causes Bus Error. #998

Open
Apidcloud opened this issue Apr 12, 2023 · 4 comments
Open
Assignees

Comments

@Apidcloud
Copy link

Apidcloud commented Apr 12, 2023

Morning!

This week someone uploaded a specific gltf that seems to cause a bus error when going through EncodeMeshToDracoBuffer (using gltf-pipeline -- CesiumGS/gltf-pipeline#608). The specific line is https://github.com/CesiumGS/gltf-pipeline/blob/901c94f360d60382dfbc8612c12130bc4992f10c/lib/compressDracoMeshes.js#L264 .

What's even worse is that listening to such signal (process.on('SIGBUS', ...) ) causes the whole process to hang -- still don't know why.

Here's some minimal js code that reproduces the issue. Remove the process.on('SIGBUS'...) to see the node process crash and show the bus error.

const fsExtra = require("fs-extra");
const processGltf = require("./lib/processGltf");
const gltf = fsExtra.readJsonSync("./broken.gltf");
const options = {
  dracoOptions: {
    compressionLevel: 10,
  },
};

processGltf(gltf, options).then(function (results) {
  fsExtra.writeJsonSync("model-draco.gltf", results.gltf);
}, function (error){
  console.log(error);
});

// not having process.on makes it crash with the bus error
process.on('SIGBUS', (signal) => {
  console.log('here', signal);
})

Running the above script with node --report-signal=SIGBUS --report-on-signal test.js also makes it hang (and doesn't create a stack trace). Same for cpu profiling, etc -- they all get hanged.

If I use draco 1.3.6, before the flags NODEJS_CATCH_EXIT and NODEJS_CATCH_REJECTION are disabled, I get the error below instead (rather than bus error). Could it be a side-effect of #629 ?

RangeError: Maximum call stack size exceeded
RangeError: Maximum call stack size exceeded
RangeError: Maximum call stack size exceeded
RangeError: Maximum call stack size exceeded
RangeError: Maximum call stack size exceeded
RangeError: Maximum call stack size exceeded
RangeError: Maximum call stack size exceeded
    at deferUnhandledRejectionCheck (/Users/luis/workspace/Git/deteleTarget/node_modules/bluebird/js/release/debuggability.js:50:9)
    at Promise._ensurePossibleRejectionHandled (/Users/luis/workspace/Git/deteleTarget/node_modules/bluebird/js/release/debuggability.js:70:5)
    at Promise._reject (/Users/luis/workspace/Git/deteleTarget/node_modules/bluebird/js/release/promise.js:694:14)
    at Promise._rejectCallback (/Users/luis/workspace/Git/deteleTarget/node_modules/bluebird/js/release/promise.js:509:10)
    at doThenable (/Users/luis/workspace/Git/deteleTarget/node_modules/bluebird/js/release/thenables.js:67:17)
    at tryConvertToPromise (/Users/luis/workspace/Git/deteleTarget/node_modules/bluebird/js/release/thenables.js:28:20)
    at Promise._resolveCallback (/Users/luis/workspace/Git/deteleTarget/node_modules/bluebird/js/release/promise.js:465:24)
    at resolve (/Users/luis/workspace/Git/deteleTarget/node_modules/bluebird/js/release/thenables.js:73:17)
    at Object.Module.then (/Users/luis/workspace/Git/deteleTarget/node_modules/draco3d/draco_encoder_nodejs.js:39:40258)
    at Object.tryCatcher (/Users/luis/workspace/Git/deteleTarget/node_modules/bluebird/js/release/util.js:16:23)
    at doThenable (/Users/luis/workspace/Git/deteleTarget/node_modules/bluebird/js/release/thenables.js:63:38)
    at tryConvertToPromise (/Users/luis/workspace/Git/deteleTarget/node_modules/bluebird/js/release/thenables.js:28:20)
    at Promise._resolveCallback (/Users/luis/workspace/Git/deteleTarget/node_modules/bluebird/js/release/promise.js:465:24)
    at resolve (/Users/luis/workspace/Git/deteleTarget/node_modules/bluebird/js/release/thenables.js:73:17)
    at Object.Module.then (/Users/luis/workspace/Git/deteleTarget/node_modules/draco3d/draco_encoder_nodejs.js:39:40258)
    at Object.tryCatcher (/Users/luis/workspace/Git/deteleTarget/node_modules/bluebird/js/release/util.js:16:23)

broken.gltf.zip

@Apidcloud Apidcloud changed the title Calling EncodeMeshToDracoBuffer causes Bus Error Potential Regression. Calling EncodeMeshToDracoBuffer causes Bus Error. Apr 12, 2023
@javagl
Copy link

javagl commented Apr 14, 2023

It might be worth mentioning that the input model is in fact invalid, because it contains an accessor that contains NaN values. So one has to expect a certain kind of error at some point (but maybe the exact error handling mechanisms have to be reviewed or changed, to prevent certain kinds of errors or crashes...)

@Apidcloud
Copy link
Author

Apidcloud commented Apr 14, 2023

A normal error would be fine and expected. But causing a bus error (since 1.3.6) and the fact that listening to such termination signal (SIGBUS) makes everything hang is worth some concern--regardless of whether the model is perfectly valid or not.

But yeah, as javagl mentioned it does contain NaN. Simply replacing them with 0 makes the problem go away. Any chance we can handle this issue in draco directly?

@donmccurdy
Copy link
Contributor

Related:

On some platforms this error surfaces an "memory access out of bounds" (included here in case others are searching for that term).

Unfortunately my hunch would be that this error occurs before the data ever reaches the Draco codebase, somewhere in the WASM API and/or generated bindings. A nicer handling of the error may be difficult. Tools like glTF Validator can catch the invalid data before processing a glTF file.

@Apidcloud
Copy link
Author

Seems similar indeed. For now yeah, gltf Validator seems the best option. Even created KhronosGroup/glTF-Validator#205 specifically for this.

Could also be a side effect or related to #629 as mentioned above.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants