react-native: Severe memory leak affecting fetch(), iOS Release Mode 0.59.0-rc.3, 0.58.6, 0.57.8

🐛 Bug Report

Every call to fetch() uses memory and does not release it. The larger the fetch response, the more memory leaked.

I’ve confirmed that this is happening in 0.57.8, 0.58.6, and 0.59.0-rc.3.
I’ve tried this with both node-fetch and whatwg-fetch and it happens using both libraries.
It is not isolated to response.blob(). It affects all types of responses.
It happens with both Release and Debug.
It happens with raw XMLHttpRequest as well as fetch.
I can confirm that it’s happening on a (real) iPhone X with iOS 12.1.4 and the Simulator for an iPhone 8 with iOS 12.1.

Comparing memory snapshots in Safari found no difference in size even while the Chrome/Xcode memory profiles showed dramatic increases in memory. I’m not great at debugging memory leaks so I could be wrong but I suspect that means it might be on the native side and not on the JS side.

To Reproduce

I’ve created an example repo to reproduce the error in 0.59.0-rc.3: https://github.com/cjroth/react-native-fetch-memory-leak. You can copy the App.js and see that it happens in previous versions as well.

Expected Behavior

After each fetch() call that is no longer referenced, the memory should return to what it was previously.

Code Example

https://github.com/cjroth/react-native-fetch-memory-leak

Environment

  React Native Environment Info:
    System:
      OS: macOS 10.14.1
      CPU: (4) x64 Intel(R) Core(TM) i7-4650U CPU @ 1.70GHz
      Memory: 16.96 MB / 8.00 GB
      Shell: 5.3 - /bin/zsh
    Binaries:
      Node: 11.10.0 - /usr/local/bin/node
      Yarn: 1.13.0 - /usr/local/bin/yarn
      npm: 6.8.0 - /usr/local/bin/npm
      Watchman: 4.9.0 - /usr/local/bin/watchman
    SDKs:
      iOS SDK:
        Platforms: iOS 12.1, macOS 10.14, tvOS 12.1, watchOS 5.1
    IDEs:
      Xcode: 10.1/10B61 - /usr/bin/xcodebuild
    npmPackages:
      react: 16.8.3 => 16.8.3 
      react-native: 0.59.0-rc.3 => 0.59.0-rc.3 
    npmGlobalPackages:
      react-native-cli: 2.0.1
      react-native-git-upgrade: 0.2.7
      react-native-rename: 2.4.0

screen shot 2019-03-06 at 9 22 40 pm

About this issue

Original URL
State: closed
Created 5 years ago
Reactions: 13
Comments: 22 (4 by maintainers)

Commits related to this issue

Release underlying resources when JS instance in GC'ed (#24745) Summary: Our Blob implementation was very problematic because it didn't release its underlying resource when the JS instance was dealoc... — committed to facebook/react-native by janicduplessis 5 years ago

Most upvoted comments

I think I’ve fixed it. Time to make my first PR into RN.

I found that the RCTNetworkTasks were not de-allocating, and that RCTBlobManager.mm was holding onto it’s blobs (NSData) forever after telling the RCTNetworking coordination code about the response.

timsawtell on Mar 10, 2019

I was able to reproduce using the iOS simulator and a variant of @cjroth s code with a five mb test file here screenshot 2019-03-07 21 12 39

noahpryor on Mar 8, 2019

This seemed to fix the blob type requests: https://github.com/timsawtell/react-native/commit/236f00a30a6f529f9c69088da3dd6cda1893d4be

However now that I’m trying out just hitting a regular content-type: “application/json” based response the memory usage issue is still there.

I’m currently debugging that use case (which I think is far more common for apps in general) and I’ll update my fork with anything I find. Since this is my first deep dive into RN networking internals, anyone with some experience please feel free to point out where I should be looking.

timsawtell on Mar 10, 2019

what is the most recent release where this did not occur for you?

brentvatne on Mar 9, 2019

How can We solve this problem…?

coocon on Apr 1, 2019

RN is supposed to use a forked version of whatwg-fetch that doesn’t use blobs by default to work around this issue. Maybe this is not working anymore for some reason?

janicduplessis on Mar 11, 2019

OK so this is a bit of a strange one with regards to solution design. The issue as I understand it is that the RCTBlobManager isn’t necessarily at fault here as I first thought. I realised that the first attempt at a fix wasn’t right - even though it solved the use case in this issue.

The problem is that whatwg-fetch isn’t necessarily concerned with what happens to the data from a request after that request is “finished”.

So RCTBlobManager keeps hold of all data from your various fetch operations in your app. It references data via a HashMap with a blobId as the key. The “bug” is that this data is not removed by the standard fetch APIs. Now that’s not fetch’s fault, it’s not RCTBlobManager’s fault - it just is 😃

So I think the approach to resolving this memory consumption issue would be to tell RCTBlobManager to release it’s data after it receives a “read” request on that data. By that point it will return the data back over the bridge to fetch in whatever encoding the programmer has asked for it. I am making an assumption here in saying that once a programmer has read the data in JS then they probably won’t be referencing that data in RCTBlobManager again, and it would be safe to release it. The APIs aren’t even there (as far as I can tell) for the programmer to even go get it again via a blobId - further reason to think this would be the safest approach.

PR coming soon after I run some more tests.

timsawtell on Mar 10, 2019