Mocha
Mocha copied to clipboard
logancollins
Crash in Javascript Core for long-running scripts.
This is something that we've been tracking for a while in various private email threads, but I thought I should actually make an issue for it. We first reproduced it in Sketch, using Cocoascript, but it also reproduces in the Cocoascript command line tool, and in the Mocha command line tool.
From the Mocha command line, this will crash:
framework("AppKit");
framework("CoreGraphics");
for (var n = 0; n < 3000; n++) {
print(n);
var path=NSBezierPath.bezierPath();
path.moveToPoint(NSMakePoint(0,0));
path.lineToPoint(NSMakePoint(10,10));
path.lineToPoint(NSMakePoint(150,200));
path.closePath();
}
From Cocoascript, something even simpler will kill it:
for (var n = 0; n < 10000; n++) {
var test = [NSString stringWithString:"1"];
print(n + ": " + test);
}
There are two weird things about the behaviour.
- every now and then, the print statement fails to convert n into a string, resulting in output like this:
2170: 1
2171: 1
2172: 1
undefined: 1
2174: 1
2175: 1
- at a later point, the MOBox which contains the NSString class is finalized. Soon afterwards, we crash somewhere like this:
#0 0x00007fff8d754f6a in JSC::JSCallbackObject<JSC::JSDestructibleObject>::call(JSC::ExecState*) ()
#1 0x00007fff8d73df30 in JSC::handleHostCall(JSC::ExecState*, JSC::JSValue, JSC::CodeSpecializationKind) ()
#2 0x00007fff8d4cbbaa in linkFor ()
#3 0x000053ceeb001964 in 0x53ceeb001964 ()
#4 0x000053cf2b000d27 in 0x53cf2b000d27 ()
#5 0x00007fff8d7af4a1 in callToJavaScript ()
#6 0x00007fff8d734823 in JSC::JITCode::execute(JSC::VM*, JSC::ProtoCallFrame*) ()
The problem seems to be centred around Javascript Core's memory management, and its interaction with MOBox.
MOBox basically exists to associate a JS object with a Cocoa object, and hold onto a reference to the Cocoa object for as long as the JS object is alive. The boxes live in a table indexed by the Cocoa object, which also allows us to look up a Cocoa object and get the corresponding JS version (if there is one).
(correct me if I'm talking rubbish here @logancollins or @ccgus)
MOBox holds a weak (assigned) reference to the JS object (a JSObjectRef), and a strong reference to the Cocoa object. When the JS object is finalized, the box should go away, and release the reference to the the cocoa object.
The problem that seems to be happening is that we're ending up with an MOBox that contains a JSObjectRef that is stale.
This could be because:
- the JSC has thrown away the JS object but failed to call the finalize callback we registered, so the ref really is stale
- a buffer overrun or similar has stomped on the JSObjectRef in the box
- something else weird...
It's tempting to blame it all on JSC, and it is certainly possible that it's a bug, but that feels a little too convenient to me.
My suspicion lies with MOFunctionArgument, because the code is a bit hairy and it uses the FFI, which involves building up various chunks of raw memory to contain the function arguments, return value, etc.
This seems to be to be prime territory for making a copy of something (eg the JSObjectRef) in a way that's not being tracked by either memory management system.
It's also prime territory for a pointer related bug causing an overwrite - either in Mocha's own code or when the FFI system kicks in (eg if it tries to write back a result into some storage that's too small).
I can't actually find any evidence that this is happening, however...
One other bit of evidence.
If we deliberately "leak" the JS object - by calling JSValueProtect on it when we make the MOBox - then we don't see the crash.
This causes the JS objects to never be finalized, so it's not that surprising that we never end up with a stale one.
It does suggest though that the stale object ref causing the crash is one of ours (if it was one that JSC had made itself, it shouldn't be affected by our change).
Unfortunately this is a bit of a Heisenbug though, so I'm not 100% convinced by this argument.
By making that call, we're messing with the memory system and changing the conditions enough that we might just be masking the bug.
We already know that a script that runs in Cocoascript can crash in Mocha and vice versa - presumably because Cocoascript is doing additional setup on the environment - so it seems that a relatively small change in the order of object allocations can change things.
When the bug does reproduce though, for a given environment, it seems to be completely deterministic. For example it will always crash on the same iteration of a loop.
As far as it being JSC - that was my initial reaction as well, but I can't get the bug to happen in Apple's Script Editor, so they must be doing something "right" there that CocoaScript isn't.
Yeah, it happens too often to be a pure JSC bug I think - web apps would be dying all over the place.
If it is a problem in JSC, it must be something quite obscure that Mocha is doing. I'm not sure that making custom JS classes and registering callbacks would fall into that category or not.
The bug also reproduces in the Mocha 2.0 branch, and it happens on both 10.9 and 10.10, though the stack trace inside the JSC code is (unsurprisingly) different.
When the bug does reproduce though, for a given environment, it seems to be completely deterministic. For example it will always crash on the same iteration of a loop.
To be more precise, it crashes exactly at the moment MOObject_finalize callback gets called. The number of iterations depends on how many JSObject's were created. As @logancollins stated here:
https://github.com/turbobabr/CocoaScript/commit/92ba1031fe1fa80fcfe6461b5cf25bdf82dd8dd8
Without protection, the finalize callback is invoked about every 1,000 iterations
So the crash happens at the moment JSC tries to run garbage collector first time during the session.
I don't think it's necessarily the first GC run is it? Plenty of other objects get finalized before it.
The only crash that I know of that is caused the first time the garbage collector one runs is the crash caused by the workaround that in that commit, if you uncomment the JSValueUnprotect.
With that code, it will indeed crash during finalization, because it's illegal to call JSValueUnprotect from within the finalize callback. That will only happen when the context is being cleaned up though, because for all other of our JS values we'll be holding an extra "reference" (via the JSValueProtect call) so they'll have been leaked and won't get GCd.
(in general @turbobabr the discussion on this issue is about the code without that patch - which is a workaround but not a proper fix as it doesn't address the underlying cause afaik)
@samdeane have a look at my PR #23 which should fix your issue. The real issue is that because you're creating "identical: according to isEqual:/hash: that the map table is giving out dealloc'd contents after the GC happens.
The patch fixes this by ensuring that isEqual: and hash: are not used in equality comparisons.
