From c647950e5e78f1bcd110111067b21daa44e4d464 Mon Sep 17 00:00:00 2001 From: "huzhanbo.luc" Date: Thu, 9 May 2024 19:55:43 -0700 Subject: [PATCH] fix fetch memory leak (#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: https://github.com/facebook/react-native/pull/44336 Test Plan: `RepeatedlyFetch` inside `XHR` example Reviewed By: javache Differential Revision: D57145270 Pulled By: NickGerleman fbshipit-source-id: afa53540e8563db4f9c6657f2dbbdff7bdfa66c0 --- .../Libraries/Blob/RCTBlobCollector.mm | 5 ++- .../Libraries/Blob/RCTBlobManager.h | 2 ++ .../Libraries/Blob/RCTBlobManager.mm | 6 ++++ .../ReactAndroid/api/ReactAndroid.api | 1 + .../react/modules/blob/BlobModule.java | 8 +++++ .../react/reactnativeblob/BlobCollector.cpp | 14 +++++++- .../jni/react/reactnativeblob/BlobCollector.h | 2 ++ .../js/examples/XHR/XHRExampleFetch.js | 32 ++++++++++++++++++- 8 files changed, 67 insertions(+), 3 deletions(-) diff --git a/packages/react-native/Libraries/Blob/RCTBlobCollector.mm b/packages/react-native/Libraries/Blob/RCTBlobCollector.mm index 9b2ca8cb3a0973..0043c5ae44c3ab 100644 --- 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(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 index bceb8eb5d81f68..38a7238380bb00 100755 --- 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 *)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 index 286710d69105cf..9d4cde7c457478 100755 --- 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 lock(_blobsMutex); + return _blobs[blobId].length; +} + - (NSData *)resolve:(NSDictionary *)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 index 1043824e5617f7..a3a849e0704358 100644 --- 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/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/modules/blob/BlobModule.java b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/modules/blob/BlobModule.java index d86962793ed51d..01b37af926888d 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/modules/blob/BlobModule.java +++ b/packages/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 index d16ba17aa4c553..9b8f80cb9eb2d8 100644 --- 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("getLengthOfBlob"); + auto length = getLengthMethod(blobModule_, jni::make_jstring(blobId_).get()); + return static_cast(length); +} + void BlobCollector::nativeInstall( jni::alias_ref, jni::alias_ref blobModule, @@ -53,7 +61,11 @@ void BlobCollector::nativeInstall( auto blobId = args[0].asString(rt).utf8(rt); auto blobCollector = std::make_shared(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 index f72e9be5ffb65b..17046b494f2664 100644 --- 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(jni::global_ref 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 index 5d386d5ba58c8c..3f49af76b483b9 100644 --- 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 { responseURL: ?string; @@ -59,6 +66,25 @@ class XHRExampleFetch extends React.Component { 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 ? ( @@ -88,6 +114,10 @@ class XHRExampleFetch extends React.Component { return ( +