fix fetch memory leak (facebook#44336)

Summary: Root cause of the fetch memory leak: The fetch requests store its result inside Blob which memory is managed by BlobCollector. On the JS engine side, the Blob is represented by an ID as JS string, and the GC don't know the size of the blob. So GC won't have interests to release the Blob. Fix: On iOS and Android, use `setExternalMemoryPressure` to acknowledge JS engine the size of Blob it holds. ## Changelog: [GENERAL] [FIXED] - fix fetch memory leak Pull Request resolved: facebook#44336 Test Plan: `RepeatedlyFetch` inside `XHR` example Reviewed By: javache Differential Revision: D57145270 Pulled By: NickGerleman fbshipit-source-id: afa53540e8563db4f9c6657f2dbbdff7bdfa66c0
rshest · May 10, 2024 · c647950 · c647950
1 parent 007ca0c
commit c647950
Show file tree

Hide file tree

Showing 8 changed files with 67 additions and 3 deletions.
diff --git a/packages/react-native/Libraries/Blob/RCTBlobCollector.mm b/packages/react-native/Libraries/Blob/RCTBlobCollector.mm
@@ -45,7 +45,10 @@
                 [blobManager](jsi::Runtime &rt, const jsi::Value &thisVal, const jsi::Value *args, size_t count) {
                   auto blobId = args[0].asString(rt).utf8(rt);
                   auto blobCollector = std::make_shared<RCTBlobCollector>(blobManager, blobId);
-                  return jsi::Object::createFromHostObject(rt, blobCollector);
+                  auto blobCollectorJsObject = jsi::Object::createFromHostObject(rt, blobCollector);
+                  blobCollectorJsObject.setExternalMemoryPressure(
+                      rt, [blobManager lengthOfBlobWithId:[NSString stringWithUTF8String:blobId.c_str()]]);
+                  return blobCollectorJsObject;
                 }));
       }
               queue:RCTJSThread];

diff --git a/packages/react-native/Libraries/Blob/RCTBlobManager.h b/packages/react-native/Libraries/Blob/RCTBlobManager.h
@@ -18,6 +18,8 @@ RCT_EXTERN void RCTEnableBlobManagerProcessingQueue(BOOL enabled);
 
 - (void)store:(NSData *)data withId:(NSString *)blobId;
 
+- (NSUInteger)lengthOfBlobWithId:(NSString *)blobId;
+
 - (NSData *)resolve:(NSDictionary<NSString *, id> *)blob;
 
 - (NSData *)resolve:(NSString *)blobId offset:(NSInteger)offset size:(NSInteger)size;

diff --git a/packages/react-native/Libraries/Blob/RCTBlobManager.mm b/packages/react-native/Libraries/Blob/RCTBlobManager.mm
@@ -98,6 +98,12 @@ - (void)store:(NSData *)data withId:(NSString *)blobId
   _blobs[blobId] = data;
 }
 
+- (NSUInteger)lengthOfBlobWithId:(NSString *)blobId
+{
+  std::lock_guard<std::mutex> lock(_blobsMutex);
+  return _blobs[blobId].length;
+}
+
 - (NSData *)resolve:(NSDictionary<NSString *, id> *)blob
 {
   NSString *blobId = [RCTConvert NSString:blob[@"blobId"]];

diff --git a/packages/react-native/ReactAndroid/api/ReactAndroid.api b/packages/react-native/ReactAndroid/api/ReactAndroid.api
@@ -3015,6 +3015,7 @@ public class com/facebook/react/modules/blob/BlobModule : com/facebook/fbreact/s
 	public fun addNetworkingHandler ()V
 	public fun addWebSocketHandler (D)V
 	public fun createFromParts (Lcom/facebook/react/bridge/ReadableArray;Ljava/lang/String;)V
+	public fun getLengthOfBlob (Ljava/lang/String;)J
 	public fun getTypedExportedConstants ()Ljava/util/Map;
 	public fun initialize ()V
 	public fun release (Ljava/lang/String;)V

diff --git a/...s/react-native/ReactAndroid/src/main/java/com/facebook/react/modules/blob/BlobModule.java b/...s/react-native/ReactAndroid/src/main/java/com/facebook/react/modules/blob/BlobModule.java
@@ -174,6 +174,14 @@ public void store(byte[] data, String blobId) {
     }
   }
 
+  @DoNotStrip
+  public long getLengthOfBlob(String blobId) {
+    synchronized (mBlobs) {
+      byte[] data = mBlobs.get(blobId);
+      return data != null ? data.length : 0;
+    }
+  }
+
   @DoNotStrip
   public void remove(String blobId) {
     synchronized (mBlobs) {

diff --git a/packages/react-native/ReactAndroid/src/main/jni/react/reactnativeblob/BlobCollector.cpp b/packages/react-native/ReactAndroid/src/main/jni/react/reactnativeblob/BlobCollector.cpp
@@ -32,6 +32,14 @@ BlobCollector::~BlobCollector() {
   });
 }
 
+size_t BlobCollector::getBlobLength() {
+  static auto getLengthMethod =
+      jni::findClassStatic(kBlobModuleJavaDescriptor)
+          ->getMethod<jlong(jstring)>("getLengthOfBlob");
+  auto length = getLengthMethod(blobModule_, jni::make_jstring(blobId_).get());
+  return static_cast<size_t>(length);
+}
+
 void BlobCollector::nativeInstall(
     jni::alias_ref<jclass>,
     jni::alias_ref<jobject> blobModule,
@@ -53,7 +61,11 @@ void BlobCollector::nativeInstall(
             auto blobId = args[0].asString(rt).utf8(rt);
             auto blobCollector =
                 std::make_shared<BlobCollector>(blobModuleRef, blobId);
-            return jsi::Object::createFromHostObject(rt, blobCollector);
+            auto blobCollectorJsObject =
+                jsi::Object::createFromHostObject(rt, blobCollector);
+            blobCollectorJsObject.setExternalMemoryPressure(
+                rt, blobCollector->getBlobLength());
+            return blobCollectorJsObject;
           }));
 }
 

diff --git a/packages/react-native/ReactAndroid/src/main/jni/react/reactnativeblob/BlobCollector.h b/packages/react-native/ReactAndroid/src/main/jni/react/reactnativeblob/BlobCollector.h
@@ -18,6 +18,8 @@ class BlobCollector : public jni::HybridClass<BlobCollector>,
   BlobCollector(jni::global_ref<jobject> blobModule, const std::string& blobId);
   ~BlobCollector();
 
+  size_t getBlobLength();
+
   static constexpr auto kJavaDescriptor =
       "Lcom/facebook/react/modules/blob/BlobCollector;";
 

diff --git a/packages/rn-tester/js/examples/XHR/XHRExampleFetch.js b/packages/rn-tester/js/examples/XHR/XHRExampleFetch.js
@@ -11,7 +11,14 @@
 'use strict';
 
 const React = require('react');
-const {Platform, StyleSheet, Text, TextInput, View} = require('react-native');
+const {
+  Button,
+  Platform,
+  StyleSheet,
+  Text,
+  TextInput,
+  View,
+} = require('react-native');
 
 class XHRExampleFetch extends React.Component<any, any> {
   responseURL: ?string;
@@ -59,6 +66,25 @@ class XHRExampleFetch extends React.Component<any, any> {
     return responseHeaders;
   }
 
+  startRepeatedlyFetch() {
+    const doRequest = () => {
+      const url =
+        'https://microsoftedge.github.io/Demos/json-dummy-data/5MB-min.json';
+      fetch(url, {
+        method: 'GET',
+        headers: {
+          Accept: 'application/json',
+          'Content-Type': 'application/json',
+        },
+      })
+        .then(() => {
+          console.log('fetch one time');
+        })
+        .catch(error => console.error(error));
+    };
+    setInterval(doRequest, 500);
+  }
+
   render(): React.Node {
     const responseURL = this.responseURL ? (
       <View style={{marginTop: 10}}>
@@ -88,6 +114,10 @@ class XHRExampleFetch extends React.Component<any, any> {
 
     return (
       <View>
+        <Button
+          title="RepeatedlyFetch"
+          onPress={() => this.startRepeatedlyFetch()}
+        />
         <Text style={styles.label}>Edit URL to submit:</Text>
         <TextInput
           returnKeyType="go"