-
-
Notifications
You must be signed in to change notification settings - Fork 817
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
Fix worker threads crash #1367
Fix worker threads crash #1367
Conversation
This property of
I suggest changing this value to:
This will cause This will permit older versions of Node to still use the existing functinality without the context-awareness. There are more details in the |
@jschlight Thanks, updated. Just a guess, but It seems that the |
Yes.
As a first step, you might consider adding a If the builds still fail, I can look into this deeper when I return to the office the week of August 17. It may require an update to |
Helper:
|
#else | ||
Napi::FunctionReference* constructor = | ||
env.GetInstanceData<Napi::FunctionReference>(); | ||
return obj.InstanceOf(constructor->Value()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You should check constructor
isn't null before dereferencing it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It should never be null though. It's set in the Init
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What if HasInstance
is called for a val
which doesn't have instance data in its env
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, the val's env would be the same env that we add the instance data to. The environment is shared by all the objects.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, there's no way of passing an object with a different env - that would have to be from a different worker thread which would require serialization.
Thank you. |
I'm using threads.js with sqlite3 (through express) inside a worker thread and it's working fine for me so far. But if I call, in the same express project, through a simple route, express crash. |
I and others have observed related crashes at: #1381 BTW unfortunately. |
Fixes #1365.