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

fetch() BlobModule.java memory leak fix not working if enable v8 js engine #36

Open
jgreen210 opened this issue Jan 8, 2020 · 5 comments

Comments

@jgreen210
Copy link

We've been seeing memory leaks from the mBlobs HashMap in BlobModule.java for some time (with JavaScriptCore). This is one of the tickets for this:

facebook/react-native#20352

We were expecting this to be fixed when we upgraded react native, since 0.61.3 added the following fix:

facebook/react-native@766f470

I.e. the js blob HostObject destructor is supposed to call remove() in BlobModule.java.

We found that, while this is fixed for JavaScriptCore, if we enabled the v8 engine (to avoid various native crashes we've been seeing in JavaScriptCore's libjsc.so), we were still getting java-heap out of memory errors. I.e. the fetch() memory leak is fixed by react native now, but only if use JavaScriptCore.

I created a small app that demonstrates this problem:

https://github.com/jgreen210/ReactNativeFetchBlobLeak/tree/v8

Setting breakpoints in BlobModule.java in android studio is a useful way to debug this:

https://github.com/facebook/react-native/blob/v0.61.5/ReactAndroid/src/main/java/com/facebook/react/modules/blob/BlobModule.java#L172
https://github.com/facebook/react-native/blob/v0.61.5/ReactAndroid/src/main/java/com/facebook/react/modules/blob/BlobModule.java#L184

I've not tried this for v8, although have used it for the hermes js engine, where there is a similar problem (facebook/hermes#164). For hermes, BlobModule.java's remove() method is not getting called.

react native 0.61.5

@jgreen210 jgreen210 changed the title fetch() BlobModule.java memory leak fix not working if enable v8 engine fetch() BlobModule.java memory leak fix not working if enable v8 js engine Jan 8, 2020
@Kudo
Copy link
Owner

Kudo commented Jan 10, 2020

@jgreen210 V8 manages its GC lifecycle internally. From my past experience, it will not GC very frequently. Since Blob is a JSI hosted object and owns an underlying native resources. JS engine has no memory usage for these native resources. If V8 does not do GC timely, the native resources may exceed the limit.

JavaScript, unfortunately, do not have RAII support.
In the meantime, please try if Blob.close() helps for your problem.
From your sample code:

    fetch(url)
      .then(response => response.blob())
      .then(blob => {
        const size = blob.size;
        blob.close();
        return size;
      })
      .then(bytes => {
        setTotalBytes(totalBytes + bytes);
      });

Or maybe you try async/await + try and finally.

@jgreen210
Copy link
Author

jgreen210 commented Jan 10, 2020

It didn't check whether this was just caused by less frequent garbage collections in v8 than JavaScriptCore. I did file facebook/react-native#27532 though, which might help in this situation.

If close() helps (I've not tried it or looked at what it does), would have to get react native to fix its json() implementation - it uses blob() internally.

@Kudo
Copy link
Owner

Kudo commented Mar 5, 2020

I can confirm that adding blob.close() can fix your OOM crash problem from your demo code.

As mentioned earlier, JS VM has no idea about how much memory cost for underlying JSI native resources, so it may not do GC timely and cause OOM crash.
JavaScriptCore AFAIK, will check the ratio of used memory and available memory frequently and do GC.
V8 seems not have similar design.

I would highly suggest applications to call blob.close(), free unused resources timely and not rely on JS GC mechanism.

BTW I have an experiment to patch fetch that will automatically call blob.close() at the end of promise chain. That is totally for fun, with some limitations and not practical in production code.
You may take a look at this commit.
Kudo/react-native@86ac876

@willholen
Copy link

The objects are alive in the Hermes heap so more frequent GCs will not help. Manually freeing the blobs is a good workaround, but the associated XMLHttpRequests are still accumulating. I'm still trying to work out the specific mechanics of this.

@jgreen210
Copy link
Author

@willholen - did you mean hermes in your above comment? (Hermes and v8 have the same symptoms, perhaps with the same cause, but this is an issue in the react-native-v8 repo.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants