Skip to content

Commit 57eb56f

Browse files
DorianMazurmeta-codesync[bot]
authored andcommitted
fix: Android OOM by eliminating intermediate buffer copies (#54854)
Summary: During JS bundle downloads from Metro, the multipart stream reader was copying each chunk into a new Buffer before passing it to listeners. For large bundles, this resulted in elevated peak memory usage due to duplicating chunk data (Okio read buffer + intermediate Buffer copy, plus downstream buffering), which can exceed emulator heap limits for large bundles. Example: #52818 Repro: #52797 ### Changes - Multipart parsing: pass a **bounded** `BufferedSource` per part (prevents reading past the part into the next boundary) and **drain unread bytes** after callbacks so listeners don’t need to fully consume the body. - BundleDownloader: keep streaming download behavior while restoring **atomic writes** (`.tmp` + rename) to avoid partial bundles on interruption. - Make Content-Type checks tolerant of parameters, parse `X-Http-Status` safely. ## Changelog: [ANDROID] [FIXED] - Reduced memory usage during JS bundle downloads by eliminating intermediate buffer copies Pull Request resolved: #54854 Test Plan: # From OSS contributor - [x] Verified multipart bundle downloads work correctly with progress callbacks displayed - [x] Verified non-multipart fallback path still functions - [x] Verified error responses are handled correctly - [x] Tested with large bundles (repro above) and confirmed reduced memory pressure and no crashes # E2E With Playground app (D102154809) and profiling {F1988731912} ### Memory regression test (Android) Built a scripted A/B harness for `BundleDownloader.downloadBundleFromURL` using an RNTester playground native module that exposes the downloader to JS and to `adb shell am broadcast`. Per iteration: force-stop the app, relaunch, capture `VmHWM` from `/proc/<pid>/status` as the pre-download high-water mark, fire the broadcast, wait for a `DOWNLOAD_COMPLETE` logcat sentinel, capture `VmHWM` and `dumpsys meminfo` again. Compared this commit against its parent (`abf4662924`) on a Samsung Galaxy S22 (`SM-S901B`, 256 MB default Java heap). **OOM regression test.** With a 200 MB synthetic JS bundle and the default 256 MB Java heap, the parent commit deterministically `OutOfMemoryError`s in `okio.Segment.<init>` from `MultipartStreamReader.readAllParts`. With this commit applied, the same download completes successfully. To get a numerical comparison at all I had to add `android:largeHeap="true"` and shrink the test bundle to 80 MB so the parent variant could finish. **Steady-state memory** — 80 MB bundle, 5 iterations per variant, `largeHeap=true`: | Variant | Avg post-download `VmHWM` | Avg delta from pre | Worst-case delta | |---|---|---|---|---| | After | 395 MB | +84 MB | +91 MB | | Before | 394 MB | +103 MB | **+196 MB** | `progressEvents = 0` for every parent-commit run confirms the parent code was the variant under test: its `headers["Content-Type"] == "application/javascript"` exact-match filter never fires against Metro's `application/javascript; charset=utf-8`. This patch tolerates the `; charset=…` parameter and emits ~150 events per download. **Per-iteration data** ``` phase iter pre_vmhwm_kb post_vmhwm_kb delta_kb duration_ms progress_events with-fix 1 313344 396796 83452 4386 149 with-fix 2 310344 388944 78600 4252 136 with-fix 3 313428 397092 83664 4602 150 with-fix 4 306804 398272 91468 4358 150 with-fix 5 309196 391764 82568 4410 149 without-fix 1 312332 393780 81448 3922 0 without-fix 2 196304 392668 196364 4621 0 without-fix 3 317616 393640 76024 4486 0 without-fix 4 314836 397812 82976 4362 0 without-fix 5 313224 392808 79584 3795 0 ``` **Existing tests.** `buck2 test fbsource//xplat/js/react-native-github/packages/react-native/ReactAndroid/src/test/java/com/facebook/react/devsupport:devsupport_MultipartStreamReaderTestAndroid` passes, including the new `testListenerDoesNotNeedToFullyReadBody` and `testHeaderNamesAreCaseInsensitive` cases that cover the bounded `BufferedSource` listener contract and case-insensitive header lookup. Reviewed By: GijsWeterings Differential Revision: D93102028 Pulled By: robhogan fbshipit-source-id: ec63d31cc6d72d2d4d852578072d810d3a54218d
1 parent 70e0382 commit 57eb56f

3 files changed

Lines changed: 247 additions & 80 deletions

File tree

packages/react-native/ReactAndroid/src/main/java/com/facebook/react/devsupport/BundleDownloader.kt

Lines changed: 50 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,6 @@ import okhttp3.Headers
2626
import okhttp3.OkHttpClient
2727
import okhttp3.Request
2828
import okhttp3.Response
29-
import okio.Buffer
3029
import okio.BufferedSource
3130
import okio.Okio
3231
import org.json.JSONException
@@ -121,11 +120,7 @@ public class BundleDownloader public constructor(private val client: OkHttpClien
121120

122121
val url = resp.request().url().toString()
123122
// Make sure the result is a multipart response and parse the boundary.
124-
var contentType = resp.header("content-type")
125-
if (contentType == null) {
126-
// fallback to empty string for nullability
127-
contentType = ""
128-
}
123+
val contentType = resp.header("content-type") ?: ""
129124
val regex = Pattern.compile("multipart/mixed;.*boundary=\"([^\"]+)\"")
130125
val match = regex.matcher(contentType)
131126
if (contentType.isNotEmpty() && match.find()) {
@@ -135,18 +130,28 @@ public class BundleDownloader public constructor(private val client: OkHttpClien
135130
// In case the server doesn't support multipart/mixed responses, fallback to
136131
// normal
137132
// download.
138-
resp.body().use { body ->
139-
if (body != null) {
133+
val body = resp.body()
134+
if (body != null) {
135+
body.use {
140136
processBundleResult(
141137
url,
142138
resp.code(),
143139
resp.headers(),
144-
body.source(),
140+
it.source(),
145141
outputFile,
146142
bundleInfo,
147143
callback,
148144
)
149145
}
146+
} else {
147+
callback.onFailure(
148+
makeGeneric(
149+
url,
150+
"Development server response body was empty.",
151+
"URL: $url",
152+
null,
153+
)
154+
)
150155
}
151156
}
152157
}
@@ -164,7 +169,8 @@ public class BundleDownloader public constructor(private val client: OkHttpClien
164169
bundleInfo: BundleInfo?,
165170
callback: DevBundleDownloadListener,
166171
) {
167-
if (response.body() == null) {
172+
val responseBody = response.body()
173+
if (responseBody == null) {
168174
callback.onFailure(
169175
DebugServerException(
170176
("""
@@ -181,27 +187,25 @@ public class BundleDownloader public constructor(private val client: OkHttpClien
181187
)
182188
return
183189
}
184-
val source = checkNotNull(response.body()?.source())
190+
191+
val source = responseBody.source()
185192
val bodyReader = MultipartStreamReader(source, boundary)
186193
val completed =
187194
bodyReader.readAllParts(
188195
object : ChunkListener {
189196
@Throws(IOException::class)
190197
override fun onChunkComplete(
191198
headers: Map<String, String>,
192-
body: Buffer,
199+
body: BufferedSource,
193200
isLastChunk: Boolean,
194201
) {
195202
// This will get executed for every chunk of the multipart response. The last chunk
196203
// (isLastChunk = true) will be the JS bundle, the other ones will be progress
197-
// events
198-
// encoded as JSON.
204+
// events encoded as JSON.
199205
if (isLastChunk) {
200206
// The http status code for each separate chunk is in the X-Http-Status header.
201-
var status = response.code()
202-
if (headers.containsKey("X-Http-Status")) {
203-
status = headers.getOrDefault("X-Http-Status", "0").toInt()
204-
}
207+
val status = headers["X-Http-Status"]?.toIntOrNull() ?: response.code()
208+
205209
processBundleResult(
206210
url,
207211
status,
@@ -211,34 +215,26 @@ public class BundleDownloader public constructor(private val client: OkHttpClien
211215
bundleInfo,
212216
callback,
213217
)
214-
} else {
215-
if (
216-
!headers.containsKey("Content-Type") ||
217-
headers["Content-Type"] != "application/json"
218-
) {
219-
return
220-
}
218+
return
219+
}
221220

222-
try {
223-
val progress = JSONObject(body.readUtf8())
224-
val status =
225-
if (progress.has("status")) progress.getString("status") else "Bundling"
226-
var done: Int? = null
227-
if (progress.has("done")) {
228-
done = progress.getInt("done")
229-
}
230-
var total: Int? = null
231-
if (progress.has("total")) {
232-
total = progress.getInt("total")
233-
}
234-
var percent: Int? = null
235-
if (progress.has("percent")) {
236-
percent = progress.getInt("percent")
237-
}
238-
callback.onProgress(status, done, total, percent)
239-
} catch (e: JSONException) {
240-
FLog.e(ReactConstants.TAG, "Error parsing progress JSON. $e")
241-
}
221+
val contentType = headers["Content-Type"] ?: return
222+
if (!isJsonContentType(contentType)) {
223+
return
224+
}
225+
226+
try {
227+
// Body is already bounded to this part; safe to read fully.
228+
val progress = JSONObject(body.readUtf8())
229+
val status =
230+
if (progress.has("status")) progress.getString("status") else "Bundling"
231+
val done: Int? = if (progress.has("done")) progress.getInt("done") else null
232+
val total: Int? = if (progress.has("total")) progress.getInt("total") else null
233+
val percent: Int? =
234+
if (progress.has("percent")) progress.getInt("percent") else null
235+
callback.onProgress(status, done, total, percent)
236+
} catch (e: JSONException) {
237+
FLog.e(ReactConstants.TAG, "Error parsing progress JSON.", e)
242238
}
243239
}
244240

@@ -247,7 +243,8 @@ public class BundleDownloader public constructor(private val client: OkHttpClien
247243
loaded: Long,
248244
total: Long,
249245
) {
250-
if ("application/javascript" == headers["Content-Type"]) {
246+
val contentType = headers["Content-Type"] ?: return
247+
if (isJavaScriptContentType(contentType)) {
251248
callback.onProgress(
252249
"Downloading",
253250
(loaded / 1024).toInt(),
@@ -258,6 +255,7 @@ public class BundleDownloader public constructor(private val client: OkHttpClien
258255
}
259256
}
260257
)
258+
261259
if (!completed) {
262260
callback.onFailure(
263261
DebugServerException(
@@ -331,7 +329,7 @@ public class BundleDownloader public constructor(private val client: OkHttpClien
331329

332330
@Throws(IOException::class)
333331
private fun storePlainJSInFile(body: BufferedSource, outputFile: File): Boolean {
334-
Okio.sink(outputFile).use { it -> body.readAll(it) }
332+
Okio.sink(outputFile).use { sink -> body.readAll(sink) }
335333
return true
336334
}
337335

@@ -348,5 +346,10 @@ public class BundleDownloader public constructor(private val client: OkHttpClien
348346
}
349347
}
350348
}
349+
350+
private fun isJsonContentType(value: String): Boolean = value.startsWith("application/json")
351+
352+
private fun isJavaScriptContentType(value: String): Boolean =
353+
value.startsWith("application/javascript")
351354
}
352355
}

packages/react-native/ReactAndroid/src/main/java/com/facebook/react/devsupport/MultipartStreamReader.kt

Lines changed: 101 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -10,10 +10,14 @@
1010
package com.facebook.react.devsupport
1111

1212
import java.io.IOException
13+
import java.util.TreeMap
1314
import kotlin.math.max
1415
import okio.Buffer
1516
import okio.BufferedSource
1617
import okio.ByteString
18+
import okio.Okio
19+
import okio.Source
20+
import okio.Timeout
1721

1822
/** Utility class to parse the body of a response of type multipart/mixed. */
1923
internal class MultipartStreamReader(
@@ -25,7 +29,7 @@ internal class MultipartStreamReader(
2529
interface ChunkListener {
2630
/** Invoked when a chunk of a multipart response is fully downloaded. */
2731
@Throws(IOException::class)
28-
fun onChunkComplete(headers: Map<String, String>, body: Buffer, isLastChunk: Boolean)
32+
fun onChunkComplete(headers: Map<String, String>, body: BufferedSource, isLastChunk: Boolean)
2933

3034
/** Invoked as bytes of the current chunk are read. */
3135
@Throws(IOException::class)
@@ -44,12 +48,16 @@ internal class MultipartStreamReader(
4448
val closeDelimiter: ByteString = ByteString.encodeUtf8("$CRLF--$boundary--$CRLF")
4549
val headersDelimiter: ByteString = ByteString.encodeUtf8(CRLF + CRLF)
4650

47-
val bufferLen = 4 * 1024
51+
// Buffer size for reading from network. Increased from 4KB to 16KB for better I/O
52+
// throughput and fewer syscalls. For a 2MB bundle: ~128 reads instead of ~512.
53+
// Memory impact is negligible (12KB increase) while I/O overhead is significantly reduced.
54+
val bufferLen = 16 * 1024
4855
var chunkStart: Long = 0
4956
var bytesSeen: Long = 0
5057
val content = Buffer()
58+
5159
var currentHeaders: Map<String, String>? = null
52-
var currentHeadersLength: Long = 0
60+
var currentBodyStartIndexInContent: Long = -1
5361

5462
while (true) {
5563
var isCloseDelimiter = false
@@ -58,6 +66,7 @@ internal class MultipartStreamReader(
5866
// to allow for the edge case when the delimiter is cut by read call.
5967
val searchStart =
6068
max((bytesSeen - closeDelimiter.size()).toDouble(), chunkStart.toDouble()).toLong()
69+
6170
var indexOfDelimiter = content.indexOf(delimiter, searchStart)
6271
if (indexOfDelimiter == -1L) {
6372
isCloseDelimiter = true
@@ -68,16 +77,17 @@ internal class MultipartStreamReader(
6877
bytesSeen = content.size()
6978

7079
if (currentHeaders == null) {
71-
val indexOfHeaders = content.indexOf(headersDelimiter, searchStart)
72-
if (indexOfHeaders >= 0) {
73-
source.read(content, indexOfHeaders)
80+
val indexOfHeadersDelimiter = content.indexOf(headersDelimiter, searchStart)
81+
if (indexOfHeadersDelimiter >= 0) {
7482
val headers = Buffer()
75-
content.copyTo(headers, searchStart, indexOfHeaders - searchStart)
76-
currentHeadersLength = headers.size() + headersDelimiter.size()
83+
content.copyTo(headers, searchStart, indexOfHeadersDelimiter - searchStart)
7784
currentHeaders = parseHeaders(headers)
85+
currentBodyStartIndexInContent =
86+
indexOfHeadersDelimiter + headersDelimiter.size().toLong()
7887
}
7988
} else {
80-
emitProgress(currentHeaders, content.size() - currentHeadersLength, false, listener)
89+
val loaded = max(0L, content.size() - currentBodyStartIndexInContent)
90+
emitProgress(currentHeaders, loaded, false, listener)
8191
}
8292

8393
val bytesRead = source.read(content, bufferLen.toLong())
@@ -92,26 +102,30 @@ internal class MultipartStreamReader(
92102

93103
// Ignore preamble
94104
if (chunkStart > 0) {
95-
val chunk = Buffer()
105+
if (currentHeaders != null && currentBodyStartIndexInContent >= 0) {
106+
val loadedFinal = max(0L, chunkEnd - currentBodyStartIndexInContent)
107+
emitProgress(currentHeaders, loadedFinal, true, listener)
108+
}
96109
content.skip(chunkStart)
97-
content.read(chunk, length)
98-
emitProgress(currentHeaders, chunk.size() - currentHeadersLength, true, listener)
99-
emitChunk(chunk, isCloseDelimiter, listener)
110+
emitChunk(content, length, isCloseDelimiter, listener)
111+
100112
currentHeaders = null
101-
currentHeadersLength = 0
113+
currentBodyStartIndexInContent = -1
102114
} else {
103115
content.skip(chunkEnd)
104116
}
105117
if (isCloseDelimiter) {
106118
return true
107119
}
120+
108121
chunkStart = delimiter.size().toLong()
109122
bytesSeen = chunkStart
110123
}
111124
}
112125

113126
private fun parseHeaders(data: Buffer): Map<String, String> {
114-
val headers: MutableMap<String, String> = mutableMapOf()
127+
// Header names are case-insensitive
128+
val headers: MutableMap<String, String> = TreeMap(String.CASE_INSENSITIVE_ORDER)
115129
val text = data.readUtf8()
116130
val lines = text.split(CRLF.toRegex()).dropLastWhile { it.isEmpty() }.toTypedArray()
117131
for (line in lines) {
@@ -126,20 +140,81 @@ internal class MultipartStreamReader(
126140
return headers
127141
}
128142

143+
/**
144+
* Emits a chunk to the listener. The `body` passed to the listener is bounded to the chunk body
145+
* bytes, so the listener cannot accidentally read into the next boundary.
146+
*
147+
* Also drains any unread body bytes after the callback to keep parsing in sync.
148+
*/
129149
@Throws(IOException::class)
130-
private fun emitChunk(chunk: Buffer, done: Boolean, listener: ChunkListener) {
150+
private fun emitChunk(
151+
content: Buffer,
152+
chunkLength: Long,
153+
done: Boolean,
154+
listener: ChunkListener,
155+
) {
131156
val marker: ByteString = ByteString.encodeUtf8(CRLF + CRLF)
132-
val indexOfMarker = chunk.indexOf(marker)
133-
if (indexOfMarker == -1L) {
134-
listener.onChunkComplete(emptyMap(), chunk, done)
135-
} else {
136-
val headers = Buffer()
137-
val body = Buffer()
138-
chunk.read(headers, indexOfMarker)
139-
chunk.skip(marker.size().toLong())
140-
chunk.readAll(body)
141-
listener.onChunkComplete(parseHeaders(headers), body, done)
157+
val indexOfMarker = content.indexOf(marker, 0)
158+
159+
if (indexOfMarker == -1L || indexOfMarker >= chunkLength) {
160+
// No headers marker found inside the chunk. Treat the entire chunk as body.
161+
val bodyLength = chunkLength
162+
val body = Okio.buffer(FixedLengthSource(content, bodyLength))
163+
try {
164+
listener.onChunkComplete(emptyMap(), body, done)
165+
} finally {
166+
drainFully(body)
167+
}
168+
return
142169
}
170+
171+
// Headers exist.
172+
val headersBuf = Buffer()
173+
content.read(headersBuf, indexOfMarker)
174+
content.skip(marker.size().toLong())
175+
val headers = parseHeaders(headersBuf)
176+
177+
val maxBodyLength = chunkLength - indexOfMarker - marker.size().toLong()
178+
val body = Okio.buffer(FixedLengthSource(content, maxBodyLength))
179+
try {
180+
listener.onChunkComplete(headers, body, done)
181+
} finally {
182+
drainFully(body)
183+
}
184+
}
185+
186+
private fun drainFully(body: BufferedSource) {
187+
// Drain remaining bytes from this part body (if listener didn't).
188+
// Use small reusable buffer to avoid unbounded memory.
189+
val tmp = Buffer()
190+
try {
191+
while (true) {
192+
val r = body.read(tmp, 8 * 1024L)
193+
if (r == -1L) break
194+
tmp.clear()
195+
}
196+
} catch (_: IOException) {
197+
// Best-effort drain; parsing will likely fail upstream anyway.
198+
}
199+
}
200+
201+
private class FixedLengthSource(
202+
private val upstream: Buffer,
203+
private var remaining: Long,
204+
) : Source {
205+
override fun read(sink: Buffer, byteCount: Long): Long {
206+
if (byteCount == 0L) return 0L
207+
if (remaining == 0L) return -1L
208+
val toRead = minOf(byteCount, remaining)
209+
val read = upstream.read(sink, toRead)
210+
if (read == -1L) return -1L
211+
remaining -= read
212+
return read
213+
}
214+
215+
override fun timeout(): Timeout = Timeout.NONE
216+
217+
override fun close() = Unit
143218
}
144219

145220
@Throws(IOException::class)

0 commit comments

Comments
 (0)