Skip to content

Commit

Permalink
Use Map instead of object in BlobRegistry (facebook#39528)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
ishikawa authored and ShevO27 committed Sep 26, 2023
1 parent 4fe01e6 commit 96ec5f1
Show file tree
Hide file tree
Showing 2 changed files with 53 additions and 9 deletions.
23 changes: 14 additions & 9 deletions packages/react-native/Libraries/Blob/BlobRegistry.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,27 +8,32 @@
* @format
*/

const registry: {[key: string]: number, ...} = {};
const registry: Map<string, number> = new Map();

const register = (id: string) => {
if (registry[id]) {
registry[id]++;
const used = registry.get(id);

if (used != null) {
registry.set(id, used + 1);
} else {
registry[id] = 1;
registry.set(id, 1);
}
};

const unregister = (id: string) => {
if (registry[id]) {
registry[id]--;
if (registry[id] <= 0) {
delete registry[id];
const used = registry.get(id);

if (used != null) {
if (used <= 1) {
registry.delete(id);
} else {
registry.set(id, used - 1);
}
}
};

const has = (id: string): number | boolean => {
return registry[id] && registry[id] > 0;
return registry.get(id) || false;
};

module.exports = {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
'use strict';

const BlobRegistry = require('../BlobRegistry');

describe('BlobRegistry', () => {
describe('register', () => {
it('does not throw error', () => {
expect(() => BlobRegistry.register('id1')).not.toThrowError();
});

it('registers new id', () => {
BlobRegistry.register('id2');
expect(BlobRegistry.has('id2')).toBeTruthy();
});
});

describe('unregister', () => {
it('does not throw error', () => {
expect(() => BlobRegistry.unregister('id3')).not.toThrowError();
});

it('remove registered id', () => {
BlobRegistry.register('id4');
BlobRegistry.unregister('id4');
expect(BlobRegistry.has('id4')).toBeFalsy();
});
});

describe('has', () => {
it('returns true for registered id', () => {
BlobRegistry.register('id5');
expect(BlobRegistry.has('id5')).toBeTruthy();
});

it('returns false for unregistered id', () => {
expect(BlobRegistry.has('id6')).toBeFalsy();
});
});
});

0 comments on commit 96ec5f1

Please sign in to comment.