galkahana / HummusJS

Node.js module for high performance creation, modification and parsing of PDF files and streams
http://www.pdfhummus.com
Other
1.15k stars 169 forks source link

Usage in multiple worker threads #364

Closed johanneslumpe closed 5 years ago

johanneslumpe commented 5 years ago

Hummus does not work within the context of worker threads in Node 11. More precisely, hummus cannot be loaded within multiple threads. After being loaded more than once it crashes the worker with Error: Module did not self-register..

There is an issue about this here: https://github.com/nodejs/node/issues/21481 It looks like native addons need to be context aware in order to be loaded successfully in multiple threads, as stated here: https://nodejs.org/docs/latest-v10.x/api/addons.html#addons_context_aware_addons

I am not sure how hard it would be to make hummus context aware, but supporting multiple threads would be great.

galkahana commented 5 years ago

Hi, 1.0.96 should take care of this. check it out.

johanneslumpe commented 5 years ago

@galkahana thanks for jumping on this! I've just tried it and while hummus now loads in multiple threads, it dies when combining multiple buffers into a final pdf:

#
# Fatal error in , line 0
# Check failed: map->instance_type() == JS_REGEXP_TYPE || map->instance_type() == JS_OBJECT_TYPE || map->instance_type() == JS_ERROR_TYPE || map->instance_type() == JS_ARRAY_TYPE || map->instance_type() == JS_API_OBJECT_TYPE || map->instance_type() == WASM_GLOBAL_TYPE || map->instance_type() == WASM_INSTANCE_TYPE || map->instance_type() == WASM_MEMORY_TYPE || map->instance_type() == WASM_MODULE_TYPE || map->instance_type() == WASM_TABLE_TYPE || map->instance_type() == JS_SPECIAL_API_OBJECT_TYPE.
#
#
#
#FailureMessage Object: 0x700006690c60

Running a single worker works fine, no errors during that operation. As soon as 2 workers are spun up, the above error occurs - not matter if a single instance or multiple instances do work. Could this be related to hummus accessing data that is somehow shared between all module instances and it has to use local versions of the types to check against? Similar to how in JS we cannot do instanceof across different host environments? In my case the error occurred when using pdfWriter.appendPDFPagesFromPDF to combine multiple PDFs.

galkahana commented 5 years ago

Not sure. PDFWriter is supposed to be thread safe in that it does not share objects with other PDFWriters. so using it within one thread, and another one within another should be fine. but i don't know if "combining buffer into a final pdf" goes into this definition. depending on what objects are used accross threads, we might have a sharing issue here (for instance, the internal object numbers allocator is single to a single instance of PDFWriter. if multiple objects are allocated at the same time...could create a problem. As for the NodeJS wrappers. I think they are done right so that no JS objects are shared between different contexts. but maybe im wrong.

better check out the usage. wanna share a sample script? i could debug it, or see where it may be breaching what sharing issues im aware of.

Gal.

johanneslumpe commented 5 years ago

Thanks for keeping an eye on this! I will try to share a repo with a runnable example tonight!

johanneslumpe commented 5 years ago

Here is the promised repo: https://github.com/johanneslumpe/hummus-worker-threads-bug The tests were run on node 11.11.0

I've noticed that I am also getting Illegal instruction: 4 after the crash, but I am not sure if that is just cropping up because of the failure. Curious to see how the test cases behave for you.

galkahana commented 5 years ago

K. actually i totally missed that i had quite a lot of globally shared (and with threads overridden incorrectly) elements. Had to be with how the node drivers for the C++ object are built. Anyways, quite a bit of work, but now with 1.0.99 it should be fine. I checked your code, and my own tests RE threads.

johanneslumpe commented 5 years ago

I just confirmed that the test cases in the repo work fine now! Awesome work, thank you so much!