audreyt / node-webworker-threads

Lightweight Web Worker API implementation with native threads
https://npmjs.org/package/webworker-threads
Other
2.3k stars 149 forks source link

segfault when trying to pass a typed array #131

Open lovasoa opened 7 years ago

lovasoa commented 7 years ago

Hi, I wanted to use this library for testing sql.js. Unfortunately, it crashed node.

Code example

var Worker = require('webworker-threads').Worker;
var worker = new Worker(function(){
  postMessage(new Uint8Array());
});

Backtrace

#0  0x000000000093ba56 in v8::Function::NewInstance(int, v8::Local<v8::Value>*) const ()
#1  0x00007ffff492d51b in BSONDeserializer::DeserializeValue(BsonType, bool) () from /tmp/ww-test/node_modules/webworker-threads/build/Release/WebWorkerThreads.node
#2  0x00007ffff492d25e in BSONDeserializer::DeserializeDocumentInternal(bool) () from /tmp/ww-test/node_modules/webworker-threads/build/Release/WebWorkerThreads.node
#3  0x00007ffff492d447 in BSONDeserializer::DeserializeDocument(bool) () from /tmp/ww-test/node_modules/webworker-threads/build/Release/WebWorkerThreads.node
#4  0x00007ffff492fe1a in Callback(uv_async_s*, int) () from /tmp/ww-test/node_modules/webworker-threads/build/Release/WebWorkerThreads.node
#5  0x00000000011b493c in ?? ()
#6  0x00000000011b4a33 in ?? ()
#7  0x00000000011c5848 in ?? ()
#8  0x00000000011b559a in uv_run ()
#9  0x0000000000fd0ce0 in node::Start(int, char**) ()
#10 0x00007ffff6b5f3f1 in __libc_start_main (main=0x75d0e0 <main>, argc=2, argv=0x7fffffffdfd8, init=<optimized out>, fini=<optimized out>, 
    rtld_fini=<optimized out>, stack_end=0x7fffffffdfc8) at ../csu/libc-start.c:291
audreyt commented 7 years ago

Yes. The bson library we use does not seem to serialize Uint8Array. Maybe it does now — if someone would like to check that'd be great.

lovasoa commented 7 years ago

I think it does: https://runkit.com/580f350772ebca0014ce054e/58338521cf2933001455b100

lovasoa commented 7 years ago

And it would be great if some safety code could be added in order to prevent the whole node process from crashing when values can't be serialized/deserialized :+1:

audreyt commented 7 years ago

OK then it's a matter of upgrading our bson.cc and bson.h as well as the glue code to the latest version of: https://github.com/mongodb/js-bson — also agreed safety code is important.

Help wanted. :-) I have very limited cycles for new features now that I'm Digital Minister of Taiwan.

lovasoa commented 7 years ago

Wouldn't there be a way to simply depend on bson, or on npm's bson module, in order not to have to upgrade it manually all the time ?

austinksmith commented 5 years ago

Hello, has there been any update to this? What I gather so far is this package does not support transferable objects and is instead attempting to serialize all data? In my hamsters.js library I do something like the following to make use of transferable objects and thus reduce communication overhead with the main thread and workers and back to the main thread.

    return hamster.postMessage(hamsterFood, this.prepareTransferBuffers(hamsterFood, habitat.transferrable));
  }

  /**
  * @function prepareTransferBuffers - Prepares transferrable buffers for faster message passing
  * @param {object} hamsterFood - Message to send to thread
  */
  prepareTransferBuffers(hamsterFood, transferrable) {
    let buffers = [];
    let key = null;
    if(transferrable) {
      for (key of Object.keys(hamsterFood)) {
        if(hamsterFood[key].buffer) {
          buffers.push(hamsterFood[key].buffer);
        } else if(Array.isArray(hamsterFood[key]) && typeof ArrayBuffer !== 'undefined') {
          buffers.push(new ArrayBuffer(hamsterFood[key]));
        }
      }
    }
    return buffers;
  }

So the idea here is that if someone passes a typed array, the library will use the buffer for that array and instead of passing it through a serializer the worker should just get a pointer reference and it should be basically a 0-copy transfer for that array.

This serialization thing seems to be causing a Node.js segmentation fault for me as well which is disappointing because ideally it wouldn't try to serialize at all.

audreyt commented 5 years ago

Would you like to give https://nodejs.org/api/worker_threads.html and try and see if it works in that context?

austinksmith commented 5 years ago

@audreyt

Thanks for the suggestion, I did just test that and while it does not suffer from the same problem with typed arrays and transferable objects. It does seem to have a bunch of other odd behavior and doesn't quite adhere to the webworker api like I hoped for. I'll have to do more debugging and validation to figure out exactly what's going on but mainly it seems that the worker constructor isn't spawning new workers on the fly nor is my worker pool working properly using that which is odd because assuming you have a Worker object it should be exactly the same regardless.