Fork me on GitHub
#planck
<
2016-06-26
>
mfikes01:06:53

@johanatan: I’m finding it difficult to find clear documentation on the threading semantics of the different entities exposed by the JavaScriptCore API. I tried your code and I am also getting SEGV on the bit of code that makes a number. Empirically I can see that making a change like this makes the SEGVs go away:

params->ctx = JSGlobalContextCreateInGroup(JSContextGetGroup(ctx), NULL);
(where previously it was passing the JSContextRef to the thread). I’ve never seen a need to do this before, but perhaps you are right, and this aspect is indeed not thread safe, and it is exhibiting issues in this case.

johanatan01:06:07

Strange. Pretty sure the number creation part was working

johanatan01:06:22

Did you try uncommenting the return NULL?

johanatan01:06:17

Also seemed like a pretty blanket statement from Apple regarding thread safety

mfikes01:06:15

Yes. I found that if I sprinkled fprintf(stderr, “here”) calls in the code that it would also crash when creating the number. But it could survive that if I was looking at it via the debugger.

mfikes01:06:30

I’m now curious if the idea of creating a new context for use in the thread leads to the overall API working. (It didn’t work for me, but I figure perhaps you don’t have it all hooked up yet for that code path.)

johanatan04:06:33

Ahh yea the code definitely isn't complete

johanatan04:06:39

Perhaps you're right

johanatan05:06:42

Did the JSObjectMakeArray also succeed with your change?

mfikes05:06:15

Yes. It did not crash at least.

johanatan05:06:50

Hmm, in that case I think I just need to add the code that actually calls the callback and we'll be done here

johanatan05:06:21

Btw, how did you discover JSGlobalContextCreateInGroup?

mfikes05:06:33

Yeah, just hook it up. JSGlobalContextCreateInGroup may be creating a context that needs to be released / freed.

mfikes05:06:37

@johanatan: This archive entry mentions it https://lists.webkit.org/pipermail/webkit-dev/2008-December/005915.html (But even that post is inconsistent with the comment’s you can see in the header file.)

mfikes05:06:42

I went in there and then concluded that perhaps the right thing to do is create a separate JSContext for use with the other thread.

mfikes05:06:09

Hence my comment that there is really no clear documentation on the thread-safety.

johanatan05:06:42

Hmm interesting. Well here's to hoping it works. Should be able to try it out tomorrow evening

johanatan18:06:05

@mfikes: My first attempt resulted in that stack trace ^. But after trying 5-10 times to reproduce, no luck.

johanatan18:06:19

Hope it isn't a heisenbug

mfikes18:06:15

Hate those 🙂

johanatan18:06:41

Ya, weird thing is it happens on (require '[planck.shell]) which shouldn't be running any of this risky native stuff

johanatan18:06:21

I remember seeing this somewhere but how do you cleanup memory for a JSStringRef (or other JSValue for that matter)?

johanatan18:06:52

[for example, a JSStringRef obtained via c_string_to_value in jsc_utils.h/c].

johanatan18:06:09

ahh, found it.

johanatan18:06:12

JSStringRelease

johanatan19:06:18

Well, it's getting further now but the SEGV still occurs. Now it's on the call to JSEvaluateScript

johanatan19:06:27

cljs.user=> (require '[planck.shell])
nil
cljs.user=> (planck.shell/sh-async "ls" :dir "/Users/jonathan/Documents" :env {"blah" "blaz"} (fn [_] 9))
ASAN:DEADLYSIGNAL
=================================================================
==52643==ERROR: AddressSanitizer: SEGV on unknown address 0x001000000011 (pc 0x7fff8ee337e1 bp 0x700000428c40 sp 0x700000428c30 T18)
    #0 0x7fff8ee337e0 in WTF::String::isolatedCopy() const & (JavaScriptCore+0x37e0)
    #1 0x7fff8ee57861 in OpaqueJSString::string() const (JavaScriptCore+0x27861)
    #2 0x7fff8ee576b8 in JSEvaluateScript (JavaScriptCore+0x276b8)
    #3 0x105ac6844 in wait_for_child (planck+0x100040844)
    #4 0x105ac6924 in thread_proc (planck+0x100040924)
    #5 0x7fff9dbc499c in _pthread_body (libsystem_pthread.dylib+0x399c)
    #6 0x7fff9dbc4919 in _pthread_start (libsystem_pthread.dylib+0x3919)
    #7 0x7fff9dbc2350 in thread_start (libsystem_pthread.dylib+0x1350)
@mfikes: ^

johanatan19:06:51

Looks like it panics trying to copy a WTF string. WTF?!?

johanatan19:06:24

Hmm, the heisenbug is also hitting with frequency > 0 now.

johanatan19:06:31

@mfikes: I've unfortunately got to run. If you (or anyone) wants to take a look, the code is here: https://github.com/johanatan/planck/commit/50156f326653bcec4f51f6c0332dd6adb4635b1b

johanatan22:06:00

@mfikes: were you able to try it? Perhaps the results are different on Linux

mfikes22:06:29

@johanatan: No, I haven’t been able to.