cc.textureCache.addImageAsync caused crash on Android, iOS, Mac

[0xJimmyBeh]

Crash happen on Android, iOS, Mac. Win32 not tested.

Crash on JSAutoCompartment ac(_cx, _obj.ref() ? _obj.ref() : global);

Tested with code below :

var imageUrlArray = ["http://www.cocos2d-x.org/s/upload/v35.jpg", "http://www.cocos2d-x.org/s/upload/testin.jpg", "http://www.cocos2d-x.org/s/upload/geometry_dash.jpg"];

for (var i = 0; i < imageUrlArray.length; i++)
{
    cc.textureCache.addImageAsync(imageUrlArray[i], this.imageLoaded, this);
}

#1507 (comment)

Updated : update issues

[pandamicro]

Delayed to v3.7

[0xJimmyBeh]

@pandamicro

Is there any temporarily solution for this issue?

Thanks.

[man2]

is this fixed yet? I can't get it to work on 3.6, is there any quick fix for this?

if there isn't, maybe I have to rollback to 3.3 where it works.

here is my stack trace

signal 11 (SIGSEGV), code 1 (SEGV_MAPERR), fault addr 00000000
Stack frame #00 pc 00a27548 /data/app-lib/com.jstest-1/libcocos2djs.so (JSAutoCompartment::JSAutoCompartment(JSContext_, JSObject_)+16): Routine JSAutoCompartment::JSAutoCompartment(JSContext_, JSObject_) at ??:?
Stack frame #1 pc 010951f4 /data/app-lib/com.jstest-1/libcocos2djs.so (__JSDownloaderDelegator::onSuccess(std::string const&, std::string const&, std::string const&)+304): Routine __JSDownloaderDelegator::onSuccess(std::string const&, std::string const&, std::string const&) at /Users/Documents/cocos/frameworks/runtime-src/proj.android/../../js-bindings/bindings/manual/extension/jsb_cocos2dx_extension_manual.cpp:976 (discriminator 4)

[stnguyen]

this is a serious bug. anybody has a workaround for it while we wait for the official fix?

[0xJimmyBeh]

Yes, this is a critical bug but been delayed to fix from v3.6 to v3.7.
I hope it can be fixed ASAP and won't be delayed to v3.8.

It looks like TextureCache::addImageAsync has more issue on Cocos2d-x
cocos2d/cocos2d-x#12021

For temporarily solution, i make the downloader become sync which has the old time app freeze issue.
Modify the file at /frameworks/js-bindings/bindings/manual/extension/jsb_cocos2dx_extension_manual.cpp

void __JSDownloaderDelegator::download(JSContext *cx, JS::HandleObject obj, const std::string &url, JS::HandleValue callback)
{
//auto t = std::thread(cx, obj, url, callback {
new __JSDownloaderDelegator(cx, obj, url, callback);
//});
//t.detach();
}

[pwang08]

Maybe I have fixed this issue. Modify the file at /frameworks/js-bindings/bindings/manual/extension/jsb_cocos2dx_extension_manual.cpp

void __JSDownloaderDelegator::downloadSync(JSContext *cx, JS::HandleObject obj, const std::string &url, JS::HandleValue callback)
{
    new __JSDownloaderDelegator(cx, obj, url, callback);
}

// jsb.loadRemoteImg(url, function(succeed, result) {})
bool js_load_remote_image(JSContext *cx, uint32_t argc, jsval *vp)
{
    if (argc == 2) {
        auto t = std::thread([cx, argc, vp]() {
            JS::CallArgs args = JS::CallArgsFromVp(argc, vp);
            JS::RootedObject obj(cx, JS_THIS_OBJECT(cx, vp));
            std::string url;
            bool ok = jsval_to_std_string(cx, args.get(0), &url);
            JS::RootedValue callback(cx, args.get(1));

            __JSDownloaderDelegator::downloadSync(cx, obj, url, callback);

            JSB_PRECONDITION2(ok, cx, false, "js_console_log : Error processing arguments");

            args.rval().setUndefined();
        });
        t.detach();
        return true;
    }

    JS_ReportError(cx, "js_load_remote_image : wrong number of arguments");
    return false;
}

Ok, it works now!

[0xJimmyBeh]

@pwang08
Does your solution have freeze issue when downloading the image?

Thanks.

[pwang08]

@Zinitter I have start a thread to process, so the solution works well.

[0xJimmyBeh]

@pwang08
Great! I will try your solution when i have time.

Between, have you test it with this code which call addImageAsync multiple times or try to load same url?
There is a "not thread safe" issue on the textureCache which failure to cache the image.

var imageUrlArray = ["http://www.cocos2d-x.org/s/upload/v35.jpg", "http://www.cocos2d-  x.org/s/upload/testin.jpg", "http://www.cocos2d-x.org/s/upload/geometry_dash.jpg"];

for (var i = 0; i < imageUrlArray.length; i++)
{
    cc.textureCache.addImageAsync(imageUrlArray[i], this.imageLoaded, this);
}

[pandamicro]

Hi, all

I finally created a proper PR for fixing this issue. This PR have refactored __JSDownloadDelegate for multithread.

cocos2d/cocos2d-x#12132

In fact, the core issue is that the creations and manipulations of js objects and values in __JSDownloadDelegate must be executed in the same thread.

[0xJimmyBeh]

@pandamicro

Thanks for the fixed.
But your PR seems got errors as it failed the check.

I tried the code and compile for Mac but got error.

void __JSDownloaderDelegator::startDownload() is not declare on jsb_cocos2dx_extension_manual.h

[pandamicro]

It passed Jenkins CI now, and you can see the declaration of startDownload here:

https://github.com/cocos2d/cocos2d-x/pull/12132/files#diff-27b4510bafdc8fe56a48aeaaaac81716R44

[0xJimmyBeh]

@pandamicro

Good job!
The solution is working on Mac, iOS, Android.
Unfortunately, i can't help to test it on Windows too.

But there is still a freeze issue on Android and iOS which cause by Image::initWithImageData as we discussed before.

So I would like to request a feature which make the Image::initWithImageData become async in Android and iOS.
By then, cc.textureCache.addImageAsync will fully async across platform.

Thanks for your effort!

[pandamicro]

@Zinitter But initWithImageData is executed outside the cocos thread, I don't understand why there is a freeze.

By the way, can you help me review the logic of my solution ?

[0xJimmyBeh]

@pandamicro

I tested the code with my project which might call cc.textureCache.addImageAsync to load image with url at the same time.

 cc.textureCache.addImageAsync(imageUrl, this.downloadCompleted, this);

Using same image url would not crash the app and able to download the image and create the sprite.

But I realised that there is slightly freeze when the callback "this.downloadCompleted" is called when success.
The freeze issue is happen on Android and iOS. Mac is fine.

I tried with 2 cases on the callback and both case have the freeze issue.

1. simply cc.log
2. create a sprite with the texture

So far in my testing, the app only crashed once. The callback is called with texture = null as show in log.
downloadCompleted texture:null
Assert failed: Invalid size
Assert failed: Invalid size
Assert failed: Invalid size
Assert failed: Invalid size
Assert failed: Could not attach texture to framebuffer

I'm not very sure how that happen, but these are the steps i tested with.
Scene 1 which call cc.textureCache.addImageAsync multiple to load different image and create the sprite
Scene 2 which do the same as Scene 1

Step 1
At Scene 1, loading image, while the image not completed download, change to Scene 2 quickly by

 cc.director.runScene(new cc.TransitionFade(transitionTime, new Scene2()));

Step 2
Repeat Step 1 when at Scene 2 to go back Scene 1

Hope these help.

I have another request, can we make cc.RenderTexture.saveToFile async so that when cc.textureCache.addImageAsync downloaded the image, then i can cache the image by save it to a file.

 cc.RenderTexture.saveToFile(imageName, cc.IMAGE_FORMAT_JPEG);

Or do we have other way to cache the image so when we reopen the app, we don't need to download the image again.

Thanks.

[pandamicro]

Thank you very much for the test ! I will look into it

Or do we have other way to cache the image so when we reopen the app, we don't need to download the image again.

I think it's more proper to directly download the file and store them in user device. For now the implementation of JSDownloadDelegate is downloading via a buffer, but Downloader supports download files too. The reason I haven't done it in this way is because I don't know whether it's desired, and I don't know where user want to store the images.

[0xJimmyBeh]

How about implementation like this

For download and use (current implementation)

 cc.textureCache.addImageAsync 

For download to file and use

 cc.textureCache.downloadImageAsync

The path to store the file by default is either

var storagePath = ((jsb.fileUtils ? jsb.fileUtils.getWritablePath() : "/"));
var storagePath = ((jsb.fileUtils ? jsb.fileUtils.getWritablePath() : "/download/"));

User can change the default storage path if they want.

Assume we downloading multiple files at the same time, i'm curious if directly download the file and store them, then straight load from the file and create a sprite, would it have the frozen issue or the time needed to process as we don't preload the resources.

Thanks.

[pandamicro]

I agree with the basic idea, but I'm afraid we won't have time to implement it very soon, because it requires an API change, we take such change very seriously

Some problem:

The path to store the file by default is either

var storagePath = ((jsb.fileUtils ? jsb.fileUtils.getWritablePath() : "/"));
var storagePath = ((jsb.fileUtils ? jsb.fileUtils.getWritablePath() : "/download/"));

That will violate end user's storage, because all cocos applications will save in the same folder by default.
So I think it should be explicitly defined by developer

Assume we downloading multiple files at the same time, i'm curious if directly download the file and store them, then straight load from the file and create a sprite, would it have the frozen issue or the time needed to process as we don't preload the resources.

I think it should be occupied by downloadImageAsync

[0xJimmyBeh]

I'm understand that Cocos team are very busy recently.

Does the Downloader supports download files is for the hotfix feature?
Can't we use the same feature as live hotfix (support image only)?

If not, do you have any temporarily solution for cc.RenderTexture.saveToFile or cc.textureCache.downloadImageAsync to avoid the frozen issue?

Cocos2d-JS is lack of multi-threading which fully utilise multi core cpu and avoid the frozen issue on coding. Cocos team should implement something to handle the multi threading issue for Cocos2d-JS.

About the storage, i don't know that will violate end user's storage. But it should not a problem to defined by developer who want to use the feature.

Thanks.

[thuydx55]

I still got crash after apply this commit cocos2d/cocos2d-x@fdec552 when offline