From 36cc71ab36aac5e5a78f2fbae44583d1df9c3cef Mon Sep 17 00:00:00 2001 From: Alec Winograd Date: Mon, 5 Dec 2022 07:22:49 -0800 Subject: [PATCH] Prevent native blob resource from being de-allocated prematurely (#31392) Summary: This PR prevents blob data from being prematurely de-allocated in native code when using slice to create views into an existing blob. Currently, whenever a new blob is created via createFromOptions, BlobManager.js creates a new blobCollector object since options.__collector is never provided. https://github.com/facebook/react-native/blob/dc80b2dcb52fadec6a573a9dd1824393f8c29fdc/Libraries/Blob/BlobManager.js#L115-L123 When the reference to a blobCollector is garbage collected, the corresponding native memory for the blob data is de-allocated. https://github.com/facebook/react-native/blob/27651720b40cab564a0cbd41be56a02584e0c73a/Libraries/Blob/RCTBlobCollector.mm#L19-L25 Since, `blob.slice()` is supposed to create a new view onto the same binary data as the original blob, we need to re-use the same collector object when slicing so that it is not GC'd until the last reference to the binary data is no longer reachable. Currently, since each blob slice gets a new blobCollector object, the memory is de-allocated when the first blob is GC'd. Fixes https://github.com/facebook/react-native/issues/29970 Fixes https://github.com/facebook/react-native/issues/27857 ## Changelog [iOS] [Fixed] - Blob data is no longer prematurely deallocated when using blob.slice Pull Request resolved: https://github.com/facebook/react-native/pull/31392 Test Plan: I could use help coming up with a test plan here. I could add a referential equality check for the blob.data.__collector in `Blob-test` but it doesn't seem quite right to be testing the implementation detail there. Reviewed By: javache Differential Revision: D41730782 Pulled By: cortinico fbshipit-source-id: 5671ae2c69908f4c9acb5d203ba198b41b421294 --- Libraries/Blob/Blob.js | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/Libraries/Blob/Blob.js b/Libraries/Blob/Blob.js index b279a030cff354..862c3f50bdcb0f 100644 --- a/Libraries/Blob/Blob.js +++ b/Libraries/Blob/Blob.js @@ -105,6 +105,12 @@ class Blob { blobId: this.data.blobId, offset, size, + /* Since `blob.slice()` creates a new view onto the same binary + * data as the original blob, we should re-use the same collector + * object so that the underlying resource gets deallocated when + * the last view into the data is released, not the first. + */ + __collector: this.data.__collector, }); }