Repository navigation
fix: image memory leaks (Android load retention, V8 image accounting, Android fd leak) - #192
Merged
Merged
Conversation
JSICallback, AsyncCallback, PromiseCallback and JSIReadFileCallback closed only the read end of the pipe that wakes the JS looper, so every async call (image loads, createImageBitmap, TextDecoder.decodeAsync, WebGPU promises) leaked a file descriptor. Each writer is done with the pipe before the reader runs, so the destructor can close both ends.
ImageAsset and ImageBitmap keep their decoded pixels in native memory that V8 cannot see. A wrapper is a few bytes on the JS heap but holds the whole RGBA buffer (4 MB for a 1366x768 image), so V8 sees no pressure to collect dropped images, and their pixels stay allocated until an unrelated GC happens to run. ObjectWrapperImpl now reports a wrapper's native bytes through v8::ExternalMemoryAccounter, and releases them in its finalizer. ImageAsset updates the figure after every load: sync loads in C++, async loads from the TS layer's completion through __syncExternalMemory. ImageBitmap reports its size when wrapped. Both runtimes (NativeScript 9.1 Android, iOS) export the accounter.
Each Android load handed Kotlin a JS-implemented Callback whose closure reached the asset. The runtime keeps such a JS object alive until Java collects its Java peer, and the Java heap rarely fills, so every loaded image stayed pinned. Reloading a three.js texture every 500 ms (apps/demo-leak) held 527 MB of native memory after a minute, unchanged by a JS GC and freed only by a Java GC. It now stays at about 69 MB. The Callback now captures only an id; the completion, and with it the asset, sits in a map until the load finishes and is then dropped. The loaders map is gone: on the C++ and iOS paths the native callback already holds a JS closure over the asset, and save() incremented it and then threw (saveCb is not registered), pinning that asset for good.
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the
 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Three fixes for image memory that grows without bound when images are loaded repeatedly, for example a texture reloaded every 500 ms with the previous one disposed.
1. Android loads pinned their asset until a Java GC
Each async
ImageAssetload on Android handed Kotlin a JS-implementedCallbackwhose closure reached the asset. The runtime keeps such an object alive until Java collects its Java peer, and the Java heap rarely fills, so every loaded image stayed reachable. A JS GC freed nothing; only a Java GC followed by a JS GC did.The callback now captures only an id. The completion, and with it the asset, sits in a map until the load finishes. The
loadersstrong-ref map is removed: it was redundant on the C++ and iOS paths, andsave()pinned its asset forever becausesaveCbis not registered.2. V8 could not see decoded image memory
An
ImageAssetwrapper is a few bytes on the JS heap but holds the whole decoded image (4 MB for 1366×768), so V8 had no reason to collect dropped ones.ImageAssetandImageBitmapwrappers now report their native bytes throughv8::ExternalMemoryAccounterand release them in the finalizer. Every NativeScript 9.1+ runtime exports the API (checked: iOS and visionOS 9.1.0, all four Android 9.1.0 ABIs).3. Android leaked a file descriptor per async native call
JSICallback,AsyncCallback,PromiseCallbackandJSIReadFileCallbackclosed only the read end of their pipe, so every async image load,createImageBitmap,TextDecoder.decodeAsyncand WebGPU promise leaked one fd.Measured
Android emulator (API 37, arm64): a three.js texture reloaded 121 times at 500 ms, native heap in MB:
Both the callback fix and the V8 accounting are needed: without the accounting, collectable images still piled up between GCs.
Not tested
The stress app the commit messages mention (
apps/demo-leak) is not part of this PR.