make module context-aware

[mac-g]

Issue or Feature

I've been experimenting with worker threads in node.js recently and noticed that the canvas module does not appear to be context-aware. In other words, the module will load/instantiate once, but as soon as another thread attempts to require canvas for it's own use, the following error is thrown:

Error: Module did not self-register.
    at Object.Module._extensions..node (internal/modules/cjs/loader.js:840:18)
    at Module.load (internal/modules/cjs/loader.js:666:32)
    at tryModuleLoad (internal/modules/cjs/loader.js:606:12)
    at Function.Module._load (internal/modules/cjs/loader.js:598:3)
    at Module.require (internal/modules/cjs/loader.js:705:19)
    at require (internal/modules/cjs/helpers.js:14:16)
    at Object.<anonymous> (~/node_modules/canvas/lib/bindings.js:3:18)
    at Module._compile (internal/modules/cjs/loader.js:799:30)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:810:10)
    at Module.load (internal/modules/cjs/loader.js:666:32)

Steps to Reproduce

In main thread...

const {Worker} = require('worker_threads');
var worker1 = new Worker('./child-thread.js'); 
var worker2 = new Worker('./child-thread.js');
// etc.

In child-thread.js...

var Canvas = require('canvas');
var canvas = Canvas.createCanvas(200, 200);
var ctx = canvas.getContext('2d');
// etc.

References

Node.js documentation: https://nodejs.org/api/addons.html#addons_context_aware_addons Similar issue: https://github.com/schroffl/node-lzo/pull/11

Environment

• node-canvas version 2.4.1
• Mac OS 10.14.3
• node version 11.12.1

[LinusU]

PRs welcome 👍

[jgoralcz]

This would be an incredible addition to node-canvas. Not sure where to fully dive in and modify the code though 🤔

[ruiwei]

I'm having the same issue. Any solution yet?

[anshul-kai]

Nope. Need to fork processes and can't use worker threads.

[siddarthareddyt]

https://github.com/Automattic/node-canvas/pull/1409 doesn't seem to fix this issue. Any plans to provide support for worker_threads?

[DePasqualeOrg]

I'm having the same issue. I'd love to be able to use node-canvas in multiple worker threads to improve performance. Anyone want to tackle this? Seems like it should be straightforward to solve:

https://github.com/nodejs/node/blob/master/doc/api/addons.md#context-aware-addons

[mistval]

This would be very useful.

[lateiskate]

I was redirected here. I'd really love for this to be a thing. Using child processes is very expensive unlike worker threads

[anna-rmrf]

This is needed

[mac-g]

Looks like I'll have a chance to maybe take a stab at this in a week while I'm on leave. No promises. Has anyone else had a chance to take a look at it? If so, I'd be great to avoid hours of re-discovery.

[lostrepo]

Maybe this will help.

I'm total noob in C++ But I use something like that to use h264 encoder in worker_threads

#include "v8.h"
#include <node.h>
#include <node_buffer.h>

using namespace v8;

Per NodeJS instance addon data

class AddonData {
    public:
    explicit AddonData(Isolate* isolate):
    wh(0) {
        // Ensure this per-addon-instance data is deleted at environment cleanup
        node::AddEnvironmentCleanupHook(isolate, DeleteInstance, this);
    }

    // Per-addon data
    int wh;
    // ... omitted
    // Node Buffer Object for YUV420 source
    MaybeLocal<Object> ab;
    // Node Buffer Object for h264 bitstream
    MaybeLocal<Object> bsiab;

    int createEncoder(){
        int rv  =   WelsCreateSVCEncoder(&encoder_);

        return encoder_ != NULL ? rv : 1;
    }

    void clean() {
        if (encoder_) {
            encoder_->Uninitialize();
            WelsDestroySVCEncoder(encoder_);
        }
    }

    static void abDeleteCb(char* data, void* hint) {
        delete reinterpret_cast<MaybeLocal<Object>*>(hint);
    }

    static void noDeleteCb(char* data, void* hint) {
        // do nothing
    }

    static void DeleteInstance(void* data) {
        AddonData* ndata    =   reinterpret_cast<AddonData*>(data);
        ndata->clean();

        delete ndata;
    }
};

Callback that accesses per NodeJS instance addon data

void encode(const FunctionCallbackInfo<Value>& info) {
    // Retrieve the per-addon-instance data
    AddonData* data =   reinterpret_cast<AddonData*>(info.Data().As<External>()->Value());

    // encode YUV420 from buf
    data->encode();
    // construct h264 bitstream from bitstream info
    data->parseBitstreamInfo();

    data->bsiab =   node::Buffer::New(info.GetIsolate(), (char*) data->bsi.data(), (size_t) data->bsiSize, AddonData::abDeleteCb, (void*) &data->bsiab);

    info.GetReturnValue().Set(data->bsiab.ToLocalChecked());
}

Per NodeJS instance module initializer (another method of initialization can be used too but this one was used in another addon that works with worker_threads and I'm total C++ noob)

extern "C" NODE_MODULE_EXPORT void
NODE_MODULE_INITIALIZER(Local<Object> exports, Local<Value> module, Local<Context> context) {
    Isolate* isolate    =   context->GetIsolate();

    // Create a new instance of `AddonData` for this instance of the addon and
    // tie its life cycle to that of the Node.js environment
    AddonData* data =   new AddonData(isolate);

    // create encoder instance
    data->createEncoder();

    // Wrap the data in a `v8::External` so we can pass it to the method we expose
    Local<External> external = External::New(isolate, data);

    // ... omitted

    exports->Set(
        context,
        String::NewFromUtf8(isolate, "encode", NewStringType::kNormal).ToLocalChecked(),
        FunctionTemplate::New(isolate, encode, external)->GetFunction(context).ToLocalChecked()
    ).FromJust();
}

[maxreisner]

I tried getting this to work with no success... If anyone who's more c++ oriented than me (and I'm not) would want some help fixing this, I'd be happy to help!

[Mjtlittle]

I hope i'm not leading anyone astray, as I have no clue how the node API bindings work with creating a context-aware module; but I found a module called 'node-lzo' that has made changes to adapt their library, that I don't see present here.

https://github.com/schroffl/node-lzo/pull/11

specifically this replacement...

NODE_MODULE_INIT(/* exports, module, context */) {
    Init(exports, context);
}

[enricodeleo]

+1

[dapriett]

+1

[lovanwubing]

+1

[dolcalmi]

I hope i'm not leading anyone astray, as I have no clue how the node API bindings work with creating a context-aware module; but I found a module called 'node-lzo' that has made changes to adapt their library, that I don't see present here.

schroffl/node-lzo#11

specifically this replacement...

NODE_MODULE_INIT(/* exports, module, context */) {
    Init(exports, context);
}

It works... I had to recompile it with the next change to make it work with worker threads

// NODE_MODULE(canvas, init);
NODE_MODULE_INIT() {
    init(exports);
}

[enricodeleo]

@dolcalmi @Mjtlittle can you please submit a pull request? I think we all would take advantage of workers support

[danielyaa5]

I tried this actually and I dont think its enough.

After I compile and run it in worker thread I get this error

TypeError: Canvas expected

I'm trying to understand the proper way to do this but I have no idea

https://nodejs.org/api/addons.html

[dteh]

@danielyaa5 I can confirm recompiling with the NODE_MODULE_INIT macro worked for me. canvas.createCanvas(200,200) returned [Canvas 200x200].

[yeldarby]

@dolcalmi or @dteh -- is a fork that has this change applied available somewhere?

Edit: yes, installed @dteh's fork from source and it worked!

brew install pkg-config cairo pango libpng jpeg giflib librsvg
npm install https://github.com/dteh/node-canvas#ISSUE-1394 --build-from-source

[effen1337]

@yeldarby I used @dteh's fork and I actually get Error [TypeError]: Canvas expected when I use a worker.

EDIT: and even FATAL ERROR: v8::HandleScope::CreateHandle() Cannot create a handle without a HandleScope

[dteh]

@effen1337 perhaps it is a platform issue? I've successfully built and deployed it on OSX 10.15.6 and CentOS 7.8.2003

[yeldarby]

I'm running it in Docker (Ubuntu 18.04 base) with no problems so far.

[yeldarby]

Hmm, I may have spoken too soon. I'm getting weird errors now too. loadImage is throwing this with some regularity... but not all the time. Not sure what the difference is between the times it works and the times it doesn't.

TypeError: Method Image.SetSource called on incompatible receiver
    at setSource (transform/node_modules/canvas/lib/image.js:91:13)
    at Image.set (transform/node_modules/canvas/lib/image.js:65:7)
    at transform/node_modules/canvas/index.js:34:15

Edit: and now I'm also getting Error [TypeError]: Canvas expected -- so weird because this was all working fin the other day. Edit: seeing these same errors in Docker as well as macOS now. So I guess this isn't working.

I ended up going back to child_process.fork instead of worker_threads

[effen1337]

After checking out this https://github.com/microsoft/node-pty/issues/405#issuecomment-622453030

Seems like even NAN_MODULE_WORKER_ENABLED doesn't get the job done on node.js 10 and up

[nickpharrison]

Is there any chance that the fix in @dteh's fork could be implemented in the main repo?

[nickpharrison]

I've been able to replicate the "Canvas expected" error @yeldarby mentioned above (on Windows) after building @dteh's fork. It occurs when calling canvas.getContext from the main thread only after the same function has previously been called from a worker thread.

[afbarbaro]

Any chance that this fix will be merged into the main repo soon? Would love to be able to use worker threads for cases where we're working with multiple canvases so each one can be operating on it's own worker thread without blocking the main thread.

[LinusU]

Any chance that this fix will be merged into the main repo soon?

I must have missed that PR, would you mind pointing me to it? 🤔

[zbjornson]

1734 was the PR, but there's more work required than what's been suggested above. I think the font registrations and saved canvas context states need per-context storage instead of static storage, might be more.

[phcreery]

Workaround as been depreciated in Electron 14. https://github.com/electron/electron/issues/18397

[niyan-ly]

Node.js worker will share memory. Native addon may run in different context, but share the same memory. The canvas expected error raise because class Canvas create a new constructor template ctor each time the addon was loaded, and bind this template to its static member constructor. Then class Context2d try to find its instance on New. But since workers are sharing memory, only worker who load canvas.node lastly will work as expected(FILO like).

And that's what context-aware means, workers run in different context, but share memory.

[LosTigeros]

If you're using it on a backend I've found and currently using alternative way. Basically I'm using BullMQ queue with Sandboxed processes to run multiple node-canvas instances. It works like a charm and was exactly what I needed.

[SonahtQ]

Any progress on that? or working workaround?

[SBRK]

I too am bothered by this limitation.

[nateGarcia]

Commenting so I can receive updates on this. This is a blocker for my visual-regression tests. Unable to run them on parallel workers.

[zbjornson]

1981 is open to address this but I haven't had time to review the recent changes. If anyone wants to try it out and/or review, that would help!

[kylerchin]

Please fix this 😭 we all need this

[DrMeepso]

Any updates?

[Balearica]

@zbjornson It looks like this issue was accidentally closed as completed due to the following sentence in #2235: We're closer to being able to close #1394. It looks like the issue is still outstanding and should be re-opened.

[BowgartField]

any updates ?

[qbqaf]

Also just ran into the issue of Module did not self-register, even in 3.0.0-rc2.

Would love to see this happen, would really make my time a lot easier ;D

[deflexable]

any updates on this?

[Zirafnik]

As @anshul-kai pointed out above, the current workaround is to use Node.js child_processes instead of worker_threads.

From what I've tested, that way works fine.