make module context-aware

Question

make module context-aware

mac-g opened this issue 6 years ago · 45 comments

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:
schroffl/node-lzo#11

Environment

node-canvas version 2.4.1
Mac OS 10.14.3
node version 11.12.1

enricodeleo commented 4 years ago

+1

dapriett commented 4 years ago

+1

lovanwubing commented 4 years ago

+1

Answer 1 · 2019-04-01T16:46:34.000Z

PRs welcome 👍

Answer 2 · 2019-06-01T23:03:56.000Z

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

Answer 3 · 2019-10-02T22:12:34.000Z

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

Answer 4 · 2019-10-03T00:24:11.000Z

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

Answer 5 · 2019-11-06T00:47:42.000Z

#1409 doesn't seem to fix this issue. Any plans to provide support for worker_threads?

Answer 6 · 2020-01-31T10:15:38.000Z

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

Answer 7 · 2020-02-15T04:59:52.000Z

This would be very useful.

Answer 8 · 2020-03-18T18:07:57.000Z

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

Answer 9 · 2020-07-04T20:08:38.000Z

This is needed

Answer 10 · 2020-07-04T21:16:14.000Z

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.

Answer 11 · 2020-07-05T16:34:17.000Z

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();
}

Answer 12 · 2020-07-23T21:23:31.000Z

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!

Answer 13 · 2020-09-07T04:17:28.000Z

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);
}

Answer 14 · 2020-10-18T19:38:55.000Z

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);
}

Answer 15 · 2020-10-18T20:29:20.000Z

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

Answer 16 · 2020-10-31T00:08:52.000Z

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

Answer 17 · 2020-11-02T14:55:49.000Z

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

Answer 18 · 2020-11-09T01:21:46.000Z

@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

Answer 19 · 2020-11-11T13:17:36.000Z

@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

Answer 20 · 2020-11-11T16:12:16.000Z

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

Answer 21 · 2020-11-11T17:27:32.000Z

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

Answer 22 · 2020-11-12T01:35:02.000Z

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

Answer 23 · 2020-11-13T12:52:21.000Z

After checking out this microsoft/node-pty#405 (comment)

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

Answer 24 · 2021-01-04T10:18:40.000Z

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

Answer 25 · 2021-01-19T05:02:36.000Z

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.

Answer 26 · 2021-04-04T16:22:41.000Z

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.

Answer 27 · 2021-04-05T11:47:19.000Z

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? 🤔

Answer 28 · 2021-04-05T18:27:31.000Z

#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.

Answer 29 · 2021-10-20T19:41:29.000Z

Workaround as been depreciated in Electron 14. electron/electron#18397

Answer 30 · 2022-01-28T10:38:18.000Z

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.

Answer 31 · 2022-01-28T21:12:41.000Z

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.

Answer 32 · 2022-03-30T12:55:10.000Z

Any progress on that? or working workaround?

Answer 33 · 2022-03-30T14:48:16.000Z

I too am bothered by this limitation.

Answer 34 · 2022-03-31T00:01:38.000Z

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

Answer 35 · 2022-03-31T00:38:03.000Z

#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!

Answer 36 · 2022-04-04T23:20:10.000Z

Please fix this 😭 we all need this

Answer 37 · 2023-06-10T22:54:34.000Z

Any updates?

Answer 38 · 2023-11-26T03:57:48.000Z

@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.

Answer 39 · 2023-12-15T15:09:38.000Z

any updates ?

Answer 40 · 2024-07-04T06:48:26.000Z

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

Answer 41 · 2024-09-20T17:20:31.000Z

any updates on this?

Answer 42 · 2024-10-12T20:19:30.000Z

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.