feat: migrate from nodejs-mobile to termux-nodejs standalone process#623
Conversation
There was a problem hiding this comment.
Code Review
This pull request refactors the Android application to run Node.js as a separate process using pre-built binaries, removing the previous JNI/CMake-based nodejs-mobile integration. Key changes include adding a Gradle script to download Node.js binaries, introducing ArchiveUtils for tar.xz extraction, and updating TerreServer to manage the process lifecycle. The review feedback highlights several critical issues: potential process hangs due to unhandled stderr and unclosed thread pools in TerreServer, a path traversal vulnerability and absolute symlink issues in ArchiveUtils, offline build failures and memory/timeout issues in the Gradle download script, and a potential memory leak in TerreViewModel from passing Activity Context into a coroutine scope.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| val executor = Executors.newSingleThreadExecutor() | ||
| executor.submit { | ||
| val homeDir = File(filesDir, "home") | ||
|
|
||
| val process = ProcessBuilder( | ||
| "$nativeLibraryDir/libnode.so", | ||
| "$nodeDir/main.js", | ||
| "--cwd", | ||
| nodeDir | ||
| ).apply { | ||
| directory(File(nodeDir)) | ||
|
|
||
| val env = environment() | ||
| env["HOME"] = homeDir.absolutePath | ||
| env["PREFIX"] = "$filesDir/usr" | ||
| env["PATH"] = "$filesDir/usr/bin" | ||
| env["LD_LIBRARY_PATH"] = "$filesDir/usr/lib:$nativeLibraryDir" | ||
| env["NODE_PATH"] = "$filesDir/usr/lib/node_modules" | ||
| env["TMPDIR"] = applicationContext.cacheDir.absolutePath | ||
| }.start() | ||
|
|
||
| nodeProcess = process | ||
|
|
||
| process.inputStream.bufferedReader().forEachLine { raw -> | ||
| if (isRunning) { | ||
| val line = raw.replace(Regex("\u001B\\[[;\\d]*m"), "") | ||
| LogStore.addLog("TERRE-NODE", line) | ||
| } | ||
| } | ||
| } else { | ||
| Log.e("StorageError", "External files directory is unavailable") | ||
|
|
||
| process.waitFor() | ||
| isRunning = false | ||
| } |
There was a problem hiding this comment.
在启动 Node.js 进程时,未对 stderr(错误输出流)进行重定向或读取。如果 Node.js 进程产生大量的错误输出或警告,可能会导致操作系统管道缓冲区溢出,从而使 Node.js 进程无限期挂起(卡死)。建议在 ProcessBuilder 中调用 redirectErrorStream(true) 将标准错误流合并到标准输出流中统一读取。
此外,每次调用 start() 时都会创建一个新的 SingleThreadExecutor,但该线程池在任务执行完毕后没有被关闭(shutdown),这可能会导致线程和内存泄漏。建议在提交任务后立即调用 executor.shutdown()(这不会中断当前正在运行的任务,但会在任务完成后释放线程池资源)。
val executor = Executors.newSingleThreadExecutor()
executor.submit {
val homeDir = File(filesDir, "home")
val process = ProcessBuilder(
"$nativeLibraryDir/libnode.so",
"$nodeDir/main.js",
"--cwd",
nodeDir
).apply {
directory(File(nodeDir))
redirectErrorStream(true)
val env = environment()
env["HOME"] = homeDir.absolutePath
env["PREFIX"] = "$filesDir/usr"
env["PATH"] = "$filesDir/usr/bin"
env["LD_LIBRARY_PATH"] = "$filesDir/usr/lib:$nativeLibraryDir"
env["NODE_PATH"] = "$filesDir/usr/lib/node_modules"
env["TMPDIR"] = applicationContext.cacheDir.absolutePath
}.start()
nodeProcess = process
process.inputStream.bufferedReader().forEachLine { raw ->
if (isRunning) {
val line = raw.replace(Regex("\\u001B\\[[;\\d]*m"), "")
LogStore.addLog("TERRE-NODE", line)
}
}
process.waitFor()
isRunning = false
}
executor.shutdown()| name | ||
| } | ||
|
|
||
| val destFile = File(toPath, relativePath) |
There was a problem hiding this comment.
在解压 tar.xz 归档文件时,直接使用 File(toPath, relativePath) 可能会引入路径遍历漏洞(Tar Slip)。如果归档文件中包含带有 ../ 的恶意文件名,可能会导致文件被写入到目标目录之外。
虽然目前解压的是应用自带的资源,但防御性编程仍然非常重要。建议在创建 destFile 后,校验其规范路径(canonicalPath)是否仍处于目标目录 toPath 之下。
val destFile = File(toPath, relativePath)
if (!destFile.canonicalPath.startsWith(File(toPath).canonicalPath)) {
throw IOException("Path traversal detected: $name")
}| val sha256Content = fetch("$baseUrl/sha256.txt") | ||
| val expectedHashes = parseSha256Content(sha256Content, targetArchitectures) |
There was a problem hiding this comment.
在 Gradle 配置/执行阶段,直接通过网络请求获取 sha256.txt。如果开发者处于离线状态(无网络连接),即使本地已经下载好了 Node.js 压缩包和解压了二进制文件,整个 Gradle 构建也会因为网络请求失败而中断。
建议增加异常捕获:如果网络请求失败,且本地对应的 libnode.so 已经存在,则跳过下载和解压步骤,允许离线构建顺利进行。
val sha256Content = try {
fetch("$baseUrl/sha256.txt")
} catch (e: Exception) {
val allExist = targetArchitectures.all { archKey ->
val abi = architectureMap[archKey]!!
File(libnodeRoot, "$abi/libnode.so").exists()
}
if (allExist) {
println("Offline mode: Node.js binaries already exist, skipping download.")
return
} else {
throw GradleException("Failed to fetch SHA256 and local binaries are missing: ${e.message}", e)
}
}
val expectedHashes = parseSha256Content(sha256Content, targetArchitectures)| var isRunning: Boolean = false | ||
| private set | ||
|
|
||
| private var nodeProcess: Process? = null |
There was a problem hiding this comment.
| val targetPath = File(destFile.parentFile, targetName).path | ||
| Os.symlink(targetPath, destFile.absolutePath) |
There was a problem hiding this comment.
在处理符号链接(Symbolic Link)时,使用 File(destFile.parentFile, targetName).path 会将相对链接解析为绝对路径。在 Android 上,如果用户使用分身应用(Dual Apps)或多用户模式,应用的私有目录路径(如 /data/user/0/... 与 /data/user/10/...)会发生变化。使用绝对路径的符号链接在其他用户或分身下会失效,导致权限拒绝或找不到文件。
建议直接使用 targetName 创建相对符号链接,这样无论应用安装在哪个用户目录下,链接都能正确解析。
| val targetPath = File(destFile.parentFile, targetName).path | |
| Os.symlink(targetPath, destFile.absolutePath) | |
| if (destFile.exists()) destFile.delete() | |
| Os.symlink(targetName, destFile.absolutePath) |
| private fun fetch(url: String): String { | ||
| println("Fetching: $url") | ||
| return URI.create(url).toURL().openStream().bufferedReader().use { it.readText() } | ||
| } | ||
|
|
||
| private fun downloadFile(url: String, target: File) { | ||
| println("Downloading: $url") | ||
| URI.create(url).toURL().openStream().use { input -> | ||
| target.outputStream().use { output -> input.copyTo(output) } | ||
| } | ||
| } |
There was a problem hiding this comment.
使用 URI.create(url).toURL().openStream() 进行网络请求和文件下载时,没有设置连接超时(Connect Timeout)和读取超时(Read Timeout)。如果网络连接不稳定或对端无响应,可能会导致 Gradle 构建无限期挂起。建议使用 URLConnection 并显式设置合理的超时时间。
private fun fetch(url: String): String {
println("Fetching: $url")
val connection = URI.create(url).toURL().openConnection()
connection.connectTimeout = 15000
connection.readTimeout = 15000
return connection.getInputStream().bufferedReader().use { it.readText() }
}
private fun downloadFile(url: String, target: File) {
println("Downloading: $url")
val connection = URI.create(url).toURL().openConnection()
connection.connectTimeout = 15000
connection.readTimeout = 15000
connection.getInputStream().use { input ->
target.outputStream().use { output -> input.copyTo(output) }
}
}| return hashes | ||
| } | ||
|
|
||
| private fun calculateSha256(file: File): String = calculateSha256(file.readBytes()) |
There was a problem hiding this comment.
calculateSha256(file) 方法中使用了 file.readBytes(),这会将整个文件一次性读入内存。Node.js 的 tarball 压缩包体积较大(几十兆字节),在内存受限的 CI 环境或低配机器上,一次性读入整包字节流可能会导致 Gradle 进程发生 OutOfMemoryError。建议改用流式读取(Streaming)的方式计算 SHA-256。
private fun calculateSha256(file: File): String {
val digest = MessageDigest.getInstance("SHA-256")
file.inputStream().use { fis ->
val buffer = ByteArray(8192)
var bytesRead = fis.read(buffer)
while (bytesRead != -1) {
digest.update(buffer, 0, bytesRead)
bytesRead = fis.read(buffer)
}
}
return digest.digest().joinToString("") { "%02x".format(it) }
}| fun startExtraction(context: Context) { | ||
| if (TerreStore.isExtracting.value) return | ||
|
|
||
| viewModelScope.launch { | ||
| TerreStore.updateIsExtracting(true) | ||
| try { | ||
| val nodeDir = context.getExternalFilesDir(null)?.absolutePath | ||
| if (nodeDir != null) { | ||
| withContext(Dispatchers.IO) { | ||
| Assets.ensureNodeRuntime(context) | ||
| Assets.extractAssets(context, nodeDir) | ||
| } | ||
| } | ||
| } catch (e: Exception) { | ||
| LogStore.addLog("TERRE", "Extraction failed: ${e.stackTraceToString()}") | ||
| TerreStore.updateIsExtracting(false) | ||
| return@launch | ||
| } | ||
| TerreStore.updateIsExtracting(false) | ||
|
|
||
| val intent = Intent(context, TerreService::class.java) | ||
| if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O) { | ||
| context.startForegroundService(intent) | ||
| } else { | ||
| context.startService(intent) | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
在 TerreViewModel.startExtraction 中,直接将传入的 context(在 Compose 中通常是 Activity Context)传入了协程作用域 viewModelScope 中。由于解压资源是耗时操作,如果在此期间 Activity 发生重建(例如屏幕旋转),旧的 Activity 实例将无法被垃圾回收,从而导致内存泄漏。
建议在方法内部首先获取 context.applicationContext,并在后续的解压和启动服务操作中全部使用 applicationContext。
fun startExtraction(context: Context) {
if (TerreStore.isExtracting.value) return
val appContext = context.applicationContext
viewModelScope.launch {
TerreStore.updateIsExtracting(true)
try {
val nodeDir = appContext.getExternalFilesDir(null)?.absolutePath
if (nodeDir != null) {
withContext(Dispatchers.IO) {
Assets.ensureNodeRuntime(appContext)
Assets.extractAssets(appContext, nodeDir)
}
}
} catch (e: Exception) {
LogStore.addLog("TERRE", "Extraction failed: ${e.stackTraceToString()}")
TerreStore.updateIsExtracting(false)
return@launch
}
TerreStore.updateIsExtracting(false)
val intent = Intent(appContext, TerreService::class.java)
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O) {
appContext.startForegroundService(intent)
} else {
appContext.startService(intent)
}
}
}…rove SHA-256 calculation fix: update TerreViewModel to use application context for service and asset extraction chore: add path traversal checks in ArchiveUtils for security
安卓版本由 nodejs-mobile JNI 调用切换到使用 ProcessBuilder 启动 termux 预构建的 nodejs