ios-jsc: memory leak when creating/wrapping native objects in worker.onmessage
Environment
Provide version numbers for the following components (information can be retrieved by running tns info in your project folder or by inspecting the package.json of the project):
- CLI: 5.3.1
- Cross-platform modules: 5.3.1
- iOS Runtime: 5.3.1
- Android Runtime (if applicable):
- Plugin(s):
Describe the bug transferring native objects to the worker thread will not release them.
To Reproduce Send pointers to native objects to the worker thread. Before that increase retain count in worker thread decrease retain count.
Observe memory in instruments tool and see it increase but never decrease. edit: running for longer time seems to clean up the memory very slowly, but afterwards it still stays with more memory than before.
https://github.com/patricklx/ns-memory-leak/blob/ios-leak/app/main-view-model.js#L28 https://github.com/patricklx/ns-memory-leak/blob/ios-leak/app/worker.js
the same also happens when not using postMessage, but using native objects, such as a NSMutableArray, to pass native objects between threads.
this prevents us to use some custom libraries or passing big native objects to the worker thread, when the quantity of this objects is large. using postMessage we need additional code to convert native to js, and decreases performance since objects are passed with JSON stringify and parse
Expected behavior memory should be released.
Sample project https://github.com/patricklx/ns-memory-leak/blob/ios-leak
About this issue
- Original URL
- State: closed
- Created 5 years ago
- Comments: 34 (15 by maintainers)
Commits related to this issue
- fix(workers): Objective-C autoreleased objects leak Each worker task should have it's own autorelease pool so that objects don't remain in memory throughout the whole worker's lifetime. refs #1137 — committed to NativeScript/ios-jsc by mbektchiev 5 years ago
- fix(workers): Memory leaks in workers' microtasks Add an `@autoreleasepool` in `microtaskRunLoopSourcePerformWork` refs https://github.com/NativeScript/ios-runtime/issues/1137#issuecomment-495250187... — committed to NativeScript/ios-jsc by mbektchiev 5 years ago
- fix(bridge): Memory leak in FFICall::initializeFFI Refactor `FFICache` to keep a copy of the arguments vector of `ffi_type*`. So far, we've leaked all pointers allocated for the local `parameterTypes... — committed to NativeScript/ios-jsc by mbektchiev 5 years ago
- fix: Additional memory leaks Move the `@autoreleasepool` from `microtaskRunLoopSourcePerformWork` to `drainMicrotasks`. This way the no already autoreleased Objective-C objects will remain alive afte... — committed to NativeScript/ios-jsc by mbektchiev 5 years ago
- fix: Prevent 2nd destructor call from ThreadSafeRefCounted It can happen in the following scenario: * `worker.js`: ```JS let o = NSNotificationCenter.defaultCenter.addObserverForNameObjectQueueUsing... — committed to NativeScript/webkit by mbektchiev 5 years ago
- fix: Prevent 2nd destructor call from ThreadSafeRefCounted It can happen in the following scenario: * `worker.js`: ```JS let o = NSNotificationCenter.defaultCenter.addObserverForNameObjectQueueUsing... — committed to NativeScript/ios-jsc by mbektchiev 5 years ago
- fix(runtime): Resend handled signal instead of exitting After introducing the our signal handler crashes have stopped being detected by Firebase. Uninstall our handler and re-send the signal so that ... — committed to NativeScript/ios-jsc by mbektchiev 5 years ago
- chore: Add unit test for workers crash on close Issue reported [here](https://github.com/NativeScript/ios-runtime/issues/1137#issuecomment-496450970) and fixed with https://github.com/NativeScript/io... — committed to NativeScript/common-runtime-tests-app by mbektchiev 5 years ago
- chore: Add unit test for workers crash on close Issue reported [here](https://github.com/NativeScript/ios-runtime/issues/1137#issuecomment-496450970) and fixed with https://github.com/NativeScript/io... — committed to NativeScript/common-runtime-tests-app by mbektchiev 5 years ago
- chore: Add unit test for workers crash on close Issue reported [here](https://github.com/NativeScript/ios-runtime/issues/1137#issuecomment-496450970) and fixed with https://github.com/NativeScript/io... — committed to NativeScript/ios-jsc by mbektchiev 5 years ago
I found out what’s causing the crash. We’ll have to think of a patch to fix it, but basically what happens is that in
JSC::VM’s destructor the JS heap is given a last chance to finalize and destroy all objects. When this reaches theNativeScript::JSBlockinstance which wraps the observer’s JS function it needs to lock the VM and perform some work. This in turn causes the reference count of the VM to rise again to 1 and when the lock is freed it once again reaches 0 causing a re-entry in VM’s destructor. It is not meant to be executed twice and subsequently crashes.One workaround we’ve tried that works is to enhance
ThreadSafeRefCounted(which is the base class of VM) to protect itself against 2nd calls of the destructor. But there might be a better solution, so we’ll consider other alternatives before rolling out this one.Regarding the
applicationmodule – it seems that it was never meant to be called from workers and so it’s definitely neither tested nor supported.mmm, I see that we have JS crashes in crashlytics before, but its also not working with 5.4.0 or 5.0.0. I’ll check more whats going on and open another issue if its something in ios-runtime. But memory is great now!
You’re right. I think that it’s best to have an
autoreleasepoolinsidedrainMicrotasks. I’ll update my pull request with this change.Hi @patricklx, I’ve found out the reason for the memory leaks and in about 1-2 hours there should be a new
@rcversion of tns-ios in NPM. Can you please get it and test the fix with it?