Skip to content

fix: Android OOM by eliminating intermediate buffer copies #54854

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.

Sign up for GitHub

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

Open
DorianMazur wants to merge 4 commits into
base: main
Choose a base branch
from
+237 −96
Open

fix: Android OOM by eliminating intermediate buffer copies #54854

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
c6d3fd6
feat: new implementation of BundleDownloader
DorianMazur Dec 11, 2025
7dd8dcc
fix: reduce copies
DorianMazur Dec 11, 2025
edc92fa
fix: typo and type
DorianMazur Dec 11, 2025
38d5673
chore: improve new implementation
DorianMazur Dec 12, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
120 changes: 57 additions & 63 deletions ...react-native/ReactAndroid/src/main/java/com/facebook/react/devsupport/BundleDownloader.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ import okhttp3.Headers
import okhttp3.OkHttpClient
import okhttp3.Request
import okhttp3.Response
import okio.Buffer
import okio.BufferedSource
import okio.Okio
import org.json.JSONException
Expand Down Expand Up @@ -120,33 +119,34 @@ public class BundleDownloader public constructor(private val client: OkHttpClien
downloadBundleFromURLCall = null

val url = resp.request().url().toString()
// Make sure the result is a multipart response and parse the boundary.
var contentType = resp.header("content-type")
if (contentType == null) {
// fallback to empty string for nullability
contentType = ""
}
val contentType = resp.header("content-type") ?: ""
val regex = Pattern.compile("multipart/mixed;.*boundary=\"([^\"]+)\"")
val match = regex.matcher(contentType)

if (contentType.isNotEmpty() && match.find()) {
val boundary = Assertions.assertNotNull(match.group(1))
processMultipartResponse(url, resp, boundary, outputFile, bundleInfo, callback)
} else {
// In case the server doesn't support multipart/mixed responses, fallback to
// normal
// download.
resp.body().use { body ->
if (body != null) {
val body = resp.body()
if (body != null) {
body.use {
processBundleResult(
url,
resp.code(),
resp.headers(),
body.source(),
it.source(),
outputFile,
bundleInfo,
callback,
)
}
} else {
callback.onFailure(
makeGeneric(url, "Development server response body was empty.", "URL: $url", null)
)
}
}
}
Expand All @@ -164,44 +164,40 @@ public class BundleDownloader public constructor(private val client: OkHttpClien
bundleInfo: BundleInfo?,
callback: DevBundleDownloadListener,
) {
if (response.body() == null) {
val responseBody = response.body()
if (responseBody == null) {
callback.onFailure(
DebugServerException(
("""
Error while reading multipart response.

Response body was empty: ${response.code()}

URL: $url


"""
.trimIndent())
)
)
return
}
val source = checkNotNull(response.body()?.source())

val source = responseBody.source()
val bodyReader = MultipartStreamReader(source, boundary)
val completed =
bodyReader.readAllParts(
object : ChunkListener {
@Throws(IOException::class)
override fun onChunkComplete(
headers: Map<String, String>,
body: Buffer,
body: BufferedSource,
isLastChunk: Boolean,
) {
// This will get executed for every chunk of the multipart response. The last chunk
// (isLastChunk = true) will be the JS bundle, the other ones will be progress
// events
// encoded as JSON.
if (isLastChunk) {
// The http status code for each separate chunk is in the X-Http-Status header.
var status = response.code()
if (headers.containsKey("X-Http-Status")) {
status = headers.getOrDefault("X-Http-Status", "0").toInt()
}
val status =
headers["X-Http-Status"]?.toIntOrNull()
?: response.code()

processBundleResult(
url,
status,
Expand All @@ -211,39 +207,29 @@ public class BundleDownloader public constructor(private val client: OkHttpClien
bundleInfo,
callback,
)
} else {
if (
!headers.containsKey("Content-Type") ||
headers["Content-Type"] != "application/json"
) {
return
}
return
}

try {
val progress = JSONObject(body.readUtf8())
val status =
if (progress.has("status")) progress.getString("status") else "Bundling"
var done: Int? = null
if (progress.has("done")) {
done = progress.getInt("done")
}
var total: Int? = null
if (progress.has("total")) {
total = progress.getInt("total")
}
callback.onProgress(status, done, total)
} catch (e: JSONException) {
FLog.e(ReactConstants.TAG, "Error parsing progress JSON. $e")
}
val contentType = headers["Content-Type"] ?: return
if (!isJsonContentType(contentType)) {
return
}

try {
// Body is already bounded to this part; safe to read fully.
val progress = JSONObject(body.readUtf8())
val status = if (progress.has("status")) progress.getString("status") else "Bundling"
val done: Int? = if (progress.has("done")) progress.getInt("done") else null
val total: Int? = if (progress.has("total")) progress.getInt("total") else null
callback.onProgress(status, done, total)
} catch (e: JSONException) {
FLog.e(ReactConstants.TAG, "Error parsing progress JSON.", e)
}
}

override fun onChunkProgress(
headers: Map<String, String>,
loaded: Long,
total: Long,
) {
if ("application/javascript" == headers["Content-Type"]) {
override fun onChunkProgress(headers: Map<String, String>, loaded: Long, total: Long) {
val contentType = headers["Content-Type"] ?: return
if (isJavaScriptContentType(contentType)) {
callback.onProgress(
"Downloading",
(loaded / 1024).toInt(),
Expand All @@ -253,17 +239,18 @@ public class BundleDownloader public constructor(private val client: OkHttpClien
}
}
)

if (!completed) {
callback.onFailure(
DebugServerException(
("""
Error while reading multipart response.

Response code: ${response.code()}

URL: $url


"""
.trimIndent())
)
Expand Down Expand Up @@ -309,7 +296,6 @@ public class BundleDownloader public constructor(private val client: OkHttpClien
val tmpFile = File(outputFile.path + ".tmp")

if (storePlainJSInFile(body, tmpFile)) {
// If we have received a new bundle from the server, move it to its final destination.
if (!tmpFile.renameTo(outputFile)) {
throw IOException("Couldn't rename $tmpFile to $outputFile")
}
Expand All @@ -326,7 +312,9 @@ public class BundleDownloader public constructor(private val client: OkHttpClien

@Throws(IOException::class)
private fun storePlainJSInFile(body: BufferedSource, outputFile: File): Boolean {
Okio.sink(outputFile).use { it -> body.readAll(it) }
Okio.sink(outputFile).use { sink ->
body.readAll(sink)
}
return true
}

Expand All @@ -343,5 +331,11 @@ public class BundleDownloader public constructor(private val client: OkHttpClien
}
}
}

private fun isJsonContentType(value: String): Boolean =
value.startsWith("application/json")

private fun isJavaScriptContentType(value: String): Boolean =
value.startsWith("application/javascript")
}
}
}
Loading
Loading