Repository navigation
[0.86] Fix premature blob deallocation during FileReader reads - #59187
dixitpriyanshu wants to merge 1 commit into
Conversation
|
Hi @dixitpriyanshu! 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! |
Backport of react#57796 to 0.86-stable. FileReader passes only `blob.data` to the native module and keeps no reference to the Blob. When the caller also drops its reference (as whatwg-fetch's readBlobAsText does for `response.json()`/`.text()`), the Blob and its BlobCollector can be GC'd while the native read is in flight. The collector's finalizer then frees the native buffer and the read rejects with "Unable to resolve data for blob" (iOS) / "The specified blob is invalid" (Android). The fix on main builds on read ids from react#57745, which 0.86 doesn't have, so it doesn't cherry-pick cleanly. This version retains the Blob on the reader for the duration of the read, releases it in `_reset()` (covering `abort()`), and settles release by identity so a stale read finishing after abort can't drop a newer read's reference.
804d71c to
1fe94ee
Compare
Summary:
Backport of #57796 (568eaa6) to
0.86-stable.FileReader.readAsText/readAsDataURL/readAsArrayBufferpass onlyblob.datato the native module and keep no reference to theBlob. whatwg-fetch reads response bodies exactly this way (readBlobAsTextkeeps only the reader), so for a plainfetch(url).then(r => r.json())the Blob and itsBlobCollectorcan be GC'd while the native read is in flight. The collector's finalizer frees the native buffer and the read rejects withUnable to resolve data for blob(iOS) /The specified blob is invalid(Android). See #56884.The commit on
maindoesn't cherry-pick cleanly: it builds on the read-id bookkeeping (_readId/_startRead) from #57745, which isn't on 0.86. This PR applies the same fix to the 0.86FileReader:_blob) for the duration of each read.abort()can't drop a newer read's reference._reset(), which coversabort().The four tests from #57796 are ported unchanged.
For context, a production app on 0.86.0 sees the iOS error from ordinary
fetch+response.json()calls (~1.5k events / 150 users over the last 90 days), and the Android message at a much larger volume.Changelog:
[GENERAL] [FIXED] - Retain Blob reference in FileReader during pending native reads to prevent premature deallocation by BlobCollector
Test Plan:
yarn jest packages/react-native/Libraries/Blob: 26 passed. The 4 new FileReader tests fail without theFileReader.jschange and pass with it.yarn flow focus-checkon both changed files: 0 errors.eslintandprettier --checkon both changed files: clean.