Skip to content

Commit 568eaa6

Browse files
heecheolmanmeta-codesync[bot]
authored andcommitted
Fix premature blob deallocation during FileReader reads (#57796)
Summary: `FileReader.readAsText` / `readAsDataURL` / `readAsArrayBuffer` pass only the plain `blob.data` descriptor to the native module and retain no reference to the `Blob` instance itself. If the caller also drops its reference, the Blob — and the `BlobCollector` attached to `blob.data.__collector` — becomes unreachable while the native read is still in flight. When GC runs in that window, the collector's finalizer unconditionally removes the bytes from the native blob store (`BlobCollector.cpp` calls `BlobModule.remove()` on Android; `RCTBlobCollector.mm` calls `[RCTBlobManager remove:]` on iOS), and the pending read rejects with **"The specified blob is invalid"** (Android) / **"Unable to resolve data for blob"** (iOS). This is not an exotic case: React Native's fetch polyfill (whatwg-fetch) reads blob bodies exactly this way — `readBlobAsText` creates a `FileReader`, calls `reader.readAsText(blob)`, and keeps a reference only to the reader. So a plain `fetch(url).then(r => r.json())`, where the `Response` is not otherwise retained, is subject to this race. This matches the symptom profile of #56884: intermittent failures under many concurrent fetches (GC pressure plus native-module thread-hop latency), affecting both platforms, and disappearing when the same flow is rewritten with `async`/`await` — the suspended frame keeps the `Response` (and therefore the Blob and its collector) reachable, which is exactly the reference this fix restores. The fix retains the Blob on the FileReader instance until the native read settles, completing the reference chain: pending native promise → callbacks → reader → `_blob` → Blob → collector. The reference is cleared when the current read settles (after the existing read-id staleness check, so a read abandoned by `abort()` cannot drop a newer read's reference) and in `abort()` itself, before the abort event is dispatched, so a read started from an abort handler is retained correctly. Memory impact is negligible: the native bytes must live until the read completes anyway — this change only guarantees they do. The root cause is in the shared JS layer, so both Android and iOS are fixed. Fixes #56884 Related prior art: #31392 fixed a different premature-deallocation path in the same subsystem (`blob.slice()` creating a second collector for the same blobId). ## Changelog: [GENERAL] [FIXED] - Retain Blob reference in FileReader during pending native reads to prevent premature deallocation by BlobCollector Pull Request resolved: #57796 Test Plan: - `yarn jest packages/react-native/Libraries/Blob/__tests__/FileReader-test.js` — 19 passed, including 4 new tests: the blob is retained while a read is pending, released on resolve / reject / `abort()`, and a stale read settling after abort does not drop a newer read's blob. - `yarn flow check` — no errors. `eslint` on both changed files — clean. - The GC race itself cannot be reproduced deterministically under Jest (it requires a real engine GC collecting the Blob between dispatch and native execution), so the unit tests assert the reference-retention behavior instead. A deterministic on-device repro is in the issue comment below / #56884. Reviewed By: javache Differential Revision: D114576384 Pulled By: fabriziocucci fbshipit-source-id: ed3f5b51f2d246e041c2b178e3eadd58aeb70038
1 parent c2dac6a commit 568eaa6

2 files changed

Lines changed: 132 additions & 0 deletions

File tree

packages/react-native/Libraries/Blob/FileReader.js

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,11 @@ class FileReader extends EventTarget {
4646
_result: ?ReaderResult;
4747
_aborted: boolean = false;
4848
_readId: number = 0;
49+
// Keep the Blob strongly referenced until the native read settles. If the
50+
// caller drops its own reference the Blob can be GC'd mid-read. Its
51+
// BlobCollector finalizer then frees the native buffer and the read fails
52+
// with "The specified blob is invalid".
53+
_blob: ?Blob;
4954

5055
constructor() {
5156
super();
@@ -56,6 +61,7 @@ class FileReader extends EventTarget {
5661
this._readyState = EMPTY;
5762
this._error = null;
5863
this._result = null;
64+
this._blob = null;
5965
}
6066

6167
_startRead(methodName: string): number {
@@ -110,12 +116,19 @@ class FileReader extends EventTarget {
110116
}
111117

112118
const readId = this._startRead('readAsArrayBuffer');
119+
// Skip if this read is no longer current: a synchronous loadstart or
120+
// readystatechange handler may have aborted or started another read during
121+
// _startRead, so setting _blob here would leak or clobber the newer read's.
122+
if (readId === this._readId) {
123+
this._blob = blob;
124+
}
113125

114126
NativeFileReaderModule.readAsDataURL(blob.data).then(
115127
(text: string) => {
116128
if (readId !== this._readId) {
117129
return;
118130
}
131+
this._blob = null;
119132

120133
const base64 = text.split(',')[1];
121134
const typedArray = toByteArray(base64);
@@ -127,6 +140,7 @@ class FileReader extends EventTarget {
127140
if (readId !== this._readId) {
128141
return;
129142
}
143+
this._blob = null;
130144
this._error = this._toDOMException(error);
131145
this._setReadyState(DONE);
132146
},
@@ -141,19 +155,24 @@ class FileReader extends EventTarget {
141155
}
142156

143157
const readId = this._startRead('readAsDataURL');
158+
if (readId === this._readId) {
159+
this._blob = blob;
160+
}
144161

145162
NativeFileReaderModule.readAsDataURL(blob.data).then(
146163
(text: string) => {
147164
if (readId !== this._readId) {
148165
return;
149166
}
167+
this._blob = null;
150168
this._result = text;
151169
this._setReadyState(DONE);
152170
},
153171
error => {
154172
if (readId !== this._readId) {
155173
return;
156174
}
175+
this._blob = null;
157176
this._error = this._toDOMException(error);
158177
this._setReadyState(DONE);
159178
},
@@ -168,19 +187,24 @@ class FileReader extends EventTarget {
168187
}
169188

170189
const readId = this._startRead('readAsText');
190+
if (readId === this._readId) {
191+
this._blob = blob;
192+
}
171193

172194
NativeFileReaderModule.readAsText(blob.data, encoding).then(
173195
(text: string) => {
174196
if (readId !== this._readId) {
175197
return;
176198
}
199+
this._blob = null;
177200
this._result = text;
178201
this._setReadyState(DONE);
179202
},
180203
error => {
181204
if (readId !== this._readId) {
182205
return;
183206
}
207+
this._blob = null;
184208
this._error = this._toDOMException(error);
185209
this._setReadyState(DONE);
186210
},
@@ -192,6 +216,10 @@ class FileReader extends EventTarget {
192216
if (this._readyState === LOADING) {
193217
this._aborted = true;
194218
this._readId++;
219+
// The abandoned read's callbacks bail out on the readId check without
220+
// clearing _blob, so release it here, before dispatching the abort event
221+
// whose handler may start a new read that sets _blob again.
222+
this._blob = null;
195223
this._setReadyState(DONE);
196224
}
197225
}

packages/react-native/Libraries/Blob/__tests__/FileReader-test.js

Lines changed: 104 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -275,4 +275,108 @@ describe('FileReader', function () {
275275
expect(() => reader.readAsText(null)).toThrow(TypeError);
276276
expect(reader.readyState).toBe(FileReader.EMPTY);
277277
});
278+
279+
it('should retain the blob until the read resolves', async () => {
280+
let resolveRead: string => void = () => {};
281+
const spy = jest
282+
.spyOn(FileReaderModuleMock, 'readAsText')
283+
.mockImplementation(
284+
() =>
285+
new Promise(resolve => {
286+
resolveRead = resolve;
287+
}),
288+
);
289+
290+
const reader = new FileReader();
291+
const blob = new Blob();
292+
const loadend = new Promise<Event>(resolve => {
293+
reader.onloadend = resolve;
294+
});
295+
reader.readAsText(blob);
296+
// $FlowFixMe[prop-missing] - accessing private state for the test
297+
expect(reader._blob).toBe(blob);
298+
299+
resolveRead('');
300+
await loadend;
301+
// $FlowFixMe[prop-missing] - accessing private state for the test
302+
expect(reader._blob).toBe(null);
303+
304+
spy.mockRestore();
305+
});
306+
307+
it('should release the blob when the read rejects', async () => {
308+
let rejectRead: Error => void = () => {};
309+
const spy = jest
310+
.spyOn(FileReaderModuleMock, 'readAsText')
311+
.mockImplementation(
312+
() =>
313+
new Promise((resolve, reject) => {
314+
rejectRead = reject;
315+
}),
316+
);
317+
318+
const reader = new FileReader();
319+
const blob = new Blob();
320+
const loadend = new Promise<Event>(resolve => {
321+
reader.onloadend = resolve;
322+
});
323+
reader.readAsText(blob);
324+
// $FlowFixMe[prop-missing] - accessing private state for the test
325+
expect(reader._blob).toBe(blob);
326+
327+
rejectRead(new Error('nope'));
328+
await loadend;
329+
// $FlowFixMe[prop-missing] - accessing private state for the test
330+
expect(reader._blob).toBe(null);
331+
332+
spy.mockRestore();
333+
});
334+
335+
it('should release the blob when a pending read is aborted', () => {
336+
const spy = jest
337+
.spyOn(FileReaderModuleMock, 'readAsText')
338+
.mockImplementation(() => new Promise(() => {}));
339+
340+
const reader = new FileReader();
341+
const blob = new Blob();
342+
reader.readAsText(blob);
343+
// $FlowFixMe[prop-missing] - accessing private state for the test
344+
expect(reader._blob).toBe(blob);
345+
346+
reader.abort();
347+
// $FlowFixMe[prop-missing] - accessing private state for the test
348+
expect(reader._blob).toBe(null);
349+
350+
spy.mockRestore();
351+
});
352+
353+
it('should keep retaining the new blob when a stale read settles after abort', async () => {
354+
const resolvers: Array<(string) => void> = [];
355+
const spy = jest
356+
.spyOn(FileReaderModuleMock, 'readAsText')
357+
.mockImplementation(
358+
() =>
359+
new Promise(resolve => {
360+
resolvers.push(resolve);
361+
}),
362+
);
363+
364+
const reader = new FileReader();
365+
const staleBlob = new Blob();
366+
reader.readAsText(staleBlob);
367+
reader.abort();
368+
369+
const newBlob = new Blob();
370+
reader.readAsText(newBlob);
371+
// $FlowFixMe[prop-missing] - accessing private state for the test
372+
expect(reader._blob).toBe(newBlob);
373+
374+
// Settle the first (aborted) read; it must not drop the new blob.
375+
resolvers[0]('');
376+
await Promise.resolve();
377+
// $FlowFixMe[prop-missing] - accessing private state for the test
378+
expect(reader._blob).toBe(newBlob);
379+
380+
spy.mockRestore();
381+
});
278382
});

0 commit comments

Comments
 (0)