-
Notifications
You must be signed in to change notification settings - Fork 24k
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
Use Map
instead of object in BlobRegistry
#39528
Use Map
instead of object in BlobRegistry
#39528
Conversation
Hi @ishikawa! Thank you for your pull request and welcome to our community. Action RequiredIn order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you. ProcessIn order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA. Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with If you have received this in error or have any questions, please contact us at cla@meta.com. Thanks! |
Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Meta Open Source project. Thanks! |
Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Meta Open Source project. Thanks! |
@NickGerleman has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
@NickGerleman merged this pull request in deca8c5. |
What version of react-native do we need to upgrade to in order to get this fix? |
Summary: issue: facebook#39441 For the following reasons, I have replaced an object used for id management inside BlobRegistry with `Map`. - The polyfill used for `fetch`, [whatwg-fetch](https://github.com/JakeChampion/fetch), returns responses as `Blob` objects. - When a `Blob` is created, it is registered with blobID in the [BlobRegistry](https://github.com/facebook/react-native/blob/main/packages/react-native/Libraries/Blob/BlobRegistry.js), which is not automatically released. - This issue was previously reported in facebook#19248 and was fixed by modifying whatwg-fetch. However, with the implementation of automatic garbage collection in facebook#24405, the implementation was reverted in commit bccc92d, returning to the original behavior. - Although facebook#24405 enables `Blob` objects to be garbage collected, the Blob IDs registered in the BlobRegistry remain, causing the count to increase each time `fetch` is called. - As a result, the `Property storage exceeds 196607 properties` error occurs To address this issue, I have modified the implementation of `BlobRegistry` to use a `Map` instead of an object. By using a `Map`, there is no limit to the number of entries. ## Changelog: [Internal] - [Fixed] - Fixed a bug that caused a "Property storage exceeds 196607 properties" error when sending a certain number of `fetch` requests. Pull Request resolved: facebook#39528 Test Plan: I've added a new tests in `packages/react-native/Libraries/Blob/__tests__/BlobRegistry-test.js` and confirmed the test pass before and after changes. ``` $ yarn run test ... Test Suites: 1 skipped, 219 passed, 219 of 220 total Tests: 2 skipped, 4017 passed, 4019 total Snapshots: 1154 passed, 1154 total Time: 10.525 s Ran all test suites. ✨ Done in 12.52s. ``` Reviewed By: javache Differential Revision: D49423213 Pulled By: NickGerleman fbshipit-source-id: d5f73d7f5e34d1d2c3969b7dfbc45d3e6196aa30
Did you get any solution as I am getting the same issue in react-native 0.72.3 |
issue: #39441
Summary:
For the following reasons, I have replaced an object used for id management inside BlobRegistry with
Map
.fetch
, whatwg-fetch, returns responses asBlob
objects.Blob
is created, it is registered with blobID in the BlobRegistry, which is not automatically released.Blob
objects to be garbage collected, the Blob IDs registered in the BlobRegistry remain, causing the count to increase each timefetch
is called.Property storage exceeds 196607 properties
error occursTo address this issue, I have modified the implementation of
BlobRegistry
to use aMap
instead of an object. By using aMap
, there is no limit to the number of entries.Changelog:
[Internal] - [Fixed] - Fixed a bug that caused a "Property storage exceeds 196607 properties" error when sending a certain number of
fetch
requests.Test Plan:
I've added a new tests in
packages/react-native/Libraries/Blob/__tests__/BlobRegistry-test.js
and confirmed the test pass before and after changes.