Skip to content
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

Memory leak on constructor error #38

Closed
elihart opened this issue Feb 28, 2014 · 5 comments
Closed

Memory leak on constructor error #38

elihart opened this issue Feb 28, 2014 · 5 comments
Labels

Comments

@elihart
Copy link

elihart commented Feb 28, 2014

When I try to create a new GifDrawable(byte[]) and it fails because the file is not a gif, I am left with a dangling byte[] of the size of the image that can never be freed. Over time this leads to large memory problems.

The problem seems to be somewhere in openByteArray(), but I haven't been able to track down the exact problem in the C code.

I load lots of images that may or may not be gifs, and I was using this as an easy test to check if they are gifs. Unfortunately the memory leak is prohibitive. I'd love some input on where to look to fix this.

@koral-- koral-- added the bug label Feb 28, 2014
@koral--
Copy link
Owner

koral-- commented Feb 28, 2014

Thanks, you are right, some memory cleanings are missing in case when opening fails. I will fix it ASAP. If you want to it yourself look at Java_pl_droidsonroids_gif_GifDrawable_openByteArray in jni/gif.c.
Before returning NULL we need to DeleteGlobalRef container->buffer and free the container like in Java_pl_droidsonroids_gif_GifDrawable_free (which is called normally from Java on cleaning up normally opened GIFs). Other functions like Java_pl_droidsonroids_gif_GifDrawable_open<type> need to be checked if there are no similar bugs inside.
Pull request will be appreciated.

@elihart
Copy link
Author

elihart commented Feb 28, 2014

Ok, good to know I'm not going crazy. Tracking down that leak was tricky.

I was already looking in Java_pl_droidsonroids_gif_GifDrawable_openByteArray but I haven't worked with C in a while and I wasn't able to find the right pointers to free.

Based on your advice I tried replacing

return open(GifFileIn, Error, container->pos, byteArrayRewindFun,
            env, metaData);

with

jint openResult = open(GifFileIn, Error, container->pos, byteArrayRewindFun,
            env, metaData);

    if (openResult == NULL) {
        (*env)->DeleteGlobalRef(env, container->buffer);
        free(container);
    }

    return openResult;

but that didn't work. I'd love to help out and submit a pull request if I can get it fixed, but unfortunately I'm struggling with C at the moment.

@koral--
Copy link
Owner

koral-- commented Feb 28, 2014

Did you recompile native sources properly?

@elihart
Copy link
Author

elihart commented Feb 28, 2014

No, I didn't! I haven't worked with jni before and wasn't aware I had to recompile it manually.

Anyway, I recompiled and now it seems to be working, thanks!

koral-- added a commit that referenced this issue Mar 1, 2014
@koral--
Copy link
Owner

koral-- commented Mar 1, 2014

Fixed for all affected sources.

@koral-- koral-- closed this as completed Mar 1, 2014
@niplus niplus mentioned this issue Aug 27, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants