-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Seg fault when accessing Context from multiple threads #5577
Comments
I'm wondering if what I'm doing wrong here is calling JsAddRef (to prevent the Context from being garbage collected) without any active context. |
Hey @crdumoul, calling @digitalinfinity might see something, but this is my current understanding of how the rental threading model for ChakraCore works. When you call JsSetCurrentContext(JS_INVALID_REFERENCE) it detaches from the current thread until a new context is set. |
To add a little more information to the backtrace, I built a debug version of libChakraCoreStatic.a. This is the updated backtrace: So, the seg fault is happening at lib/Common/Memory/RecyclerWriteBarrierManager.cpp:331 Also, I should mention that I'm using the v1.10.1 tag in git. |
This looks to me like it's an issue with @digitalinfinity do you think this is another case where the field annotations should be removed / marked as |
I'm surprised that this hasn't come up before, since this appears to have been there for years. I do see that the Perhaps instead we should have the write barriers check if the index is in bounds? Although that will definitely add some overhead. |
This might be what I was missing: https://github.com/Microsoft/ChakraCore/blob/master/lib/Common/Memory/RecyclerWriteBarrierManager.cpp#L52-L55 There is some per-thread handling of the card table, and it may be that was somehow missed on the second thread, so its stack isn't accounted for. |
I've just put out a PR which I think will fix the issue for you. Are you able to pull my branch and try it out yourself @crdumoul ? |
@MSLaguana, thanks. I should be able to try that branch tomorrow, and I'll let you know how it goes. |
@MSLaguana those changes seem to have fixed the problem. |
Great, thanks for confirming that! |
… thread that interacts with the JSRT Merge pull request #5580 from MSLaguana:updateStackCardTable The card table has code to handle stack allocations, but this code was only run on the main thread and on, our own background threads. Threads trying to use the JSRT could end up getting segfaults from write barriers from Fields on stack variables that weren't known to the write barrier. Fixes #5577
I've got a test program that does the following in two separate threads. I understand that the runtime and context can only be accessed by one thread at a time.
In the main thread, create a runtime and a context, and execute some JavaScript code to add a function called "square" to the global object. The JavaScript code being run is very simple:
function square(number) { return number * number; }
The API calls in the main thread are the following, in this order:
JsCreateRuntime
JsCreateContext
JsAddRef: passing in the JsContextRef so the Context won't be garbage collected.
JsSetCurrentContext
JsCreateExternalArrayBuffer: create a buffer that points to the JavaScript code to be run.
JsCreateString: create a string for the URL parameter to JsRun
JsRun: run the JavaScript code to create the "square" function.
JsSetCurrentContext(JS_INVALID_REFERENCE)
After that a second thread is spawned. This second thread will be calling the "square" function. It makes the following API calls, in this order:
JsSetCurrentContext
JsGetGlobalObject
JsCreatePropertyId
At this point the second thread occasionally seg faults (SIGSEGV) in the call to JsCreatePropertyId.
The back trace looks like:
#0 0x000055e69854223e in Memory::RecyclerWriteBarrierManager::WriteBarrier(void*) ()
#1 0x000055e69857d6e7 in ThreadContext::GetOrAddPropertyId(char16_t const*, int, Js::PropertyRecord const**) ()
#2 0x000055e6985698ae in Js::ScriptContext::GetOrAddPropertyRecord(char16_t const*, int, Js::PropertyRecord const**) ()
#3 0x000055e6984b252b in JsGetPropertyIdFromNameInternal(char16_t const*, unsigned long, void**)::{lambda(Js::ScriptContext*)#1} ContextAPINoScriptWrapper_Core<_JsErrorCode ContextAPINoScriptWrapper_NoRecord<JsGetPropertyIdFromNameInternal(char16_t const*, unsigned long, void**)::{lambda(Js::ScriptContext*)#1}>(JsGetPropertyIdFromNameInternal(char16_t const*, unsigned long, void**)::{lambda(Js::ScriptContext*)#1}, bool, bool)::{lambda(Js::ScriptContext*)#1}>(_JsErrorCode, bool, bool) ()
#4 0x000055e6984af6cb in JsCreatePropertyId ()
This test program is written in Rust, which is why I'm not just including the source code, but instead trying to describe the API calls I'm making.
I'd appreciate any help in figuring out what I'm doing wrong here.
The text was updated successfully, but these errors were encountered: