Javascript GC crashes

cocos2d-x JavaScript
#1

Is anyone else seeing these crashes when you have an update loop scheduled for a node? They happen pretty randomly but almost always seem to be in an update function (which may just be coincidence because it is called the most often).

#0 0x0011a295 in js::gc::Cell::getAllocKind() const ()
#1 0x0039b707 in js::gc::MarkGCThingRoot(JSTracer**, void****, char const**) ()
#2 0x00179b85 in js::MarkRuntime(JSTracer*, bool) ()
#3 0x0017dc15 in IncrementalCollectSlice(JSRuntime*, long long, js::gcreason::Reason, js::JSGCInvocationKind) ()
#4 0x0017ce1a in GCCycle(JSRuntime*, bool, long long, js::JSGCInvocationKind, js::gcreason::Reason) ()
#5 0x001794fb in Collect(JSRuntime*, bool, long long, js::JSGCInvocationKind, js::gcreason::Reason) ()
#6 0x00178128 in js::GCSlice(JSRuntime*, js::JSGCInvocationKind, js::gcreason::Reason, long long) ()
#7 0x00141b00 in js_InvokeOperationCallback(JSContext*) ()
#8 0x00141b8d in js_HandleExecutionInterrupt(JSContext*) ()
#9 0x001b6d8a in js::Interpret(JSContext**, js::StackFrame**, js::InterpMode) ()
#10 0x001b44c9 in js::RunScript(JSContext**, JSScript**, js::StackFrame*) ()
#11 0x001dc108 in SendToGenerator(JSContext**, JSGeneratorOp, JSObject**, JSGenerator*, JS::Value const&) ()
#12 0x001dd5c1 in generator_next_impl(JSContext*, JS::CallArgs) ()
#13 0x001db3ee in generator_next(JSContext**, unsigned int, JS::Value**) ()
#14 0x001d2d0c in js::CallJSNative(JSContext*, int (*)(JSContext**, unsigned int, JS::Value**), JS::CallArgs const&) ()
#15 0x001d04f9 in js::InvokeKernel(JSContext*, JS::CallArgs, js::MaybeConstruct) ()
#16 0x001d076e in js::Invoke(JSContext**, JS::Value const&, JS::Value const&, unsigned int, JS::Value**, JS::Value*) ()
#17 0x001da959 in js_IteratorMore(JSContext*, JS::Handle<JSObject*>, JS::MutableHandleJS::Value) ()
#18 0x001b992a in js::Interpret(JSContext**, js::StackFrame**, js::InterpMode) ()
#19 0x001b44c9 in js::RunScript(JSContext**, JSScript**, js::StackFrame*) ()
#20 0x001d05c0 in js::InvokeKernel(JSContext*, JS::CallArgs, js::MaybeConstruct) ()
#21 0x001d076e in js::Invoke(JSContext**, JS::Value const&, JS::Value const&, unsigned int, JS::Value**, JS::Value*) ()
#22 0x00112c82 in JS_CallFunctionName ()
#23 0x00e690cd in executeJSFunctionWithName at /javascript/proj.ios/…/bindings/ScriptingCore.cpp:157
#24 0x00e6a06f in ScriptingCore::executeSchedule(cocos2d::CCTimer**, float, cocos2d::CCNode**) at /javascript/proj.ios/…/bindings/ScriptingCore.cpp:715
#25 0x00ec537b in cocos2d::CCScheduler::update(float) at /cocos2dx/proj.ios/…/CCScheduler.cpp:813
#26 0x00eb9895 in cocos2d::CCDirector::drawScene() at /cocos2dx/proj.ios/…/CCDirector.cpp:214
#27 0x00ebbd9b in cocos2d::CCDisplayLinkDirector::mainLoop() at /cocos2dx/proj.ios/…/CCDirector.cpp:944
#28 0x00f2a335 in -[CCDirectorCaller doCaller:] at /cocos2dx/proj.ios/…/platform/ios/CCDirectorCaller.mm:93
#29 0x023912d2 in CA::Display::DisplayLink::dispatch(unsigned long long, unsigned long long) ()

#2

Hello, could you provide a demo to reproduce this issue?

#3

amitt mahajan wrote:

Is anyone else seeing these crashes when you have an update loop scheduled for a node? They happen pretty randomly but almost always seem to be in an update function (which may just be coincidence because it is called the most often).
>

We think we’ve tracked down our memory problems to a bug with the JS garbage collection for callback functions.

Repro case:

create a scroll view with an anonymous function as the onScrollCallback:

var owner = this;
var scroll = cc.ScrollView.create(...);
(set up scroll view)
scroll.setScrollCallback(function() {
   log("onScroll");
   owner.onScroll();
});

then force the GC to run:

__jsc__.garbageCollect();

The next time you scroll the scrollView, you get a bad access error. Since the JS code is keeping no references to the scroll listener, the GC call will clean it up. This results in a crash to home screen.

SpiderMonkey provides support for c++ objects keeping JS objects in memory via the “JS_Add*Root” commands (https://developer.mozilla.org/en-US/docs/SpiderMonkey/JSAPI_Reference/JS_AddRoot). I’m going to be experimenting with adding those calls to the create / destroy of the JS_CallbackWrapper to try and fix it.

Has anyone else run into similar issues, or have any advice about implementation?

#4

Hi Mark Henderson,
Which version you are using?
The latest release didn’t bind CCScrollVIew. Where did you find that?
I just bound CCScrollView and CCTableView on the github (https://github.com/cocos2d/cocos2d-x/pull/2015) .

Mark Henderson wrote:

amitt mahajan wrote:
> Is anyone else seeing these crashes when you have an update loop scheduled for a node? They happen pretty randomly but almost always seem to be in an update function (which may just be coincidence because it is called the most often).
>
>
>
We think we’ve tracked down our memory problems to a bug with the JS garbage collection for callback functions.
>
>
Repro case:
>
create a scroll view with an anonymous function as the onScrollCallback:
>
[…]
>
then force the GC to run:
>
[…]
>
The next time you scroll the scrollView, you get a bad access error. Since the JS code is keeping no references to the scroll listener, the GC call will clean it up. This results in a crash to home screen.
>
>
SpiderMonkey provides support for c++ objects keeping JS objects in memory via the “JS_Add*Root” commands (https://developer.mozilla.org/en-US/docs/SpiderMonkey/JSAPI_Reference/JS_AddRoot). I’m going to be experimenting with adding those calls to the create / destroy of the JS_CallbackWrapper to try and fix it.
>
Has anyone else run into similar issues, or have any advice about implementation?

#5

James Chen wrote:

Hi Mark Henderson,
Which version you are using?
The latest release didn’t bind CCScrollVIew. Where did you find that?
I just bound CCScrollView and CCTableView on the github (https://github.com/cocos2d/cocos2d-x/pull/2015) .
>

Hi James,

I’d forgotten that we bound scrollView manually. We started from cocos2d-x v2.1beta3-x-2.1.0, but we’ve ported in a few fixes from master since then.

I believe the same problem will exist for anywhere cocos2dx uses the JSCallbackWrapper (or its inheritors). Search for “setJSCallbackFunc” in cocos2d_specifics.cpp to see a few examples. Probably the most common one would be the binding for ccNode.scheduleOnce().

I managed to fix our problems by changing JSCallbackWrapper::setJSCallbackFunc to :

void JSCallbackWrapper::setJSCallbackFunc(jsval func) {
    jsCallback = func;
    JS_AddValueRoot(ScriptingCore::getInstance()->getGlobalContext(), &jsCallback);
}

and the JSCallbackWrapper destructor to:

virtual ~JSCallbackWrapper(void) {
   JS_RemoveValueRoot(ScriptingCore::getInstance()->getGlobalContext(), &jsCallback);
}

I’m going to be checking to make sure that doesn’t cause any other memory problems, then probably submitting a pull request.

I’m also seeing that a lot of CCActions (possibly all of them) stay around even after forcing a GC (on both the C++ and the JS side). I’m going to be focusing on that today to try and figure out if it’s an engine issue or something we’re doing wrong.

#6

Hey guy,
This bug was fixed before. Look at this file on github (https://github.com/cocos2d/cocos2d-x/blob/master/scripting/javascript/bindings/cocos2d_specifics.cpp).

#7

James Chen wrote:

Hey guy,
This bug was fixed before. Look at this file on github (https://github.com/cocos2d/cocos2d-x/blob/master/scripting/javascript/bindings/cocos2d_specifics.cpp).

That fixes the problem for scrollView by not using the JSCallbackWrapper class. Other places in the engine still use the wrapper, and we’ve bound several libraries internally using it.

EDIT This issue was actually fixed in commit https://github.com/dumganhar/cocos2d-x/commit/c037d156529a1c32845d9649729c6dc8181b3be1 - part of a checkin fixing particle effects crashes.

#8

Hi, Mark

That fixes the problem for scrollView by not using the JSCallbackWrapper class. Other places in the engine still use the wrapper, and we’ve bound several libraries internally using it.

Could you tell me how to reproduce the issue of GC crash in the latest source on the github? Thanks.

#9

James Chen wrote:

Hi, Mark
>
> That fixes the problem for scrollView by not using the JSCallbackWrapper class. Other places in the engine still use the wrapper, and we’ve bound several libraries internally using it.
>
Could you tell me how to reproduce the issue of GC crash in the latest source on the github? Thanks.

I think I looked at the wrong thing when I looked at the link you posted - I saw the scroll view bindings but didn’t see the changes to JSCallbackWrapper. When I reviewed it a couple of hours ago I saw that the issue was fixed in the commit I edited into my earlier post.

TL;DR: issue was already fixed on master, but I missed that when I reviewed the latest source.

#10

Yes, i think this issue was fixed some months ago. I don’t know why you still got this problem. So could you try the source codes on the github?
Feel free to give me the your feedback. Thanks.

Mark Henderson wrote:

James Chen wrote:
> Hi, Mark
>
> > That fixes the problem for scrollView by not using the JSCallbackWrapper class. Other places in the engine still use the wrapper, and we’ve bound several libraries internally using it.
>
> Could you tell me how to reproduce the issue of GC crash in the latest source on the github? Thanks.
>
I think I looked at the wrong thing when I looked at the link you posted - I saw the scroll view bindings but didn’t see the changes to JSCallbackWrapper. When I reviewed it a couple of hours ago I saw that the issue was fixed in the commit I edited into my earlier post.
>
TL;DR: issue was already fixed on master, but I missed that when I reviewed the latest source.

#11

James Chen wrote:

Yes, i think this issue was fixed some months ago. I don’t know why you still got this problem. So could you try the source codes on the github?
Feel free to give me the your feedback. Thanks.
>

We had the problem because we don’t want to upgrade mid project, so we’re still on an older version of cocos2dx :) There are too many breaking changes to reasonably update a game that’s almost complete and functional with the older release.
If the commit that fixed this issue included ‘memory’, ‘gc’, or other relevant keywords it probably would have helped as well ‘some improvements to JS bindings’ is a rather nebulous and non-searchable commit message.

Thanks for the quick replies!

#12

If the commit that fixed this issue included ‘memory’, ‘gc’, or other relevant keywords it probably would have helped as well - ‘some improvements to JS bindings’ is a rather nebulous and non-searchable commit message.

Thanks for your suggestion. Yes, that’s no easy to search. But we used this title was because there were lots of fixes in this issue.
If i remember correctly, the detail informations that issue fixed were at the content of it.

#13

James Chen wrote:

Thanks for your suggestion. Yes, that’s no easy to search. But we used this title was because there were lots of fixes in this issue.
If i remember correctly, the detail informations that issue fixed were at the content of it.

Is there a way to search through those commit details? I searched over the commit messages when I first started looking into this bug, but aside from clicking into each individual commit I’m not aware of a way to search through commit details.

#14

@Mark, you could search through the commit details.

#15

Guys, I may be having some problem that is related to this issue/fix: http://cocos2d-x.org/boards/20/topics/35033

I could not get the NDK_STACK with the lines that clear… I am using addr2line and I got just 2:

#0 004b1194 in void MarkInternal(JSTracer**, JSObject***)
#1 004ac43c in js::gc::MarkRuntime(JSTracer*, bool)

… it is kind of random in my game, and I could not trace the origin :confused: