Opened on 03/29/2017 at 11:15:40 AM
Closed on 03/30/2017 at 07:28:31 PM
#5053 closed change (fixed)
Release JsValues explicitly
| Reported by: | asmirnov | Assignee: | |
|---|---|---|---|
| Priority: | P2 | Milestone: | |
| Module: | Libadblockplus-Android | Keywords: | |
| Cc: | sergz, fhd | Blocked By: | |
| Blocking: | #5026 | Platform: | Android |
| Ready: | yes | Confidential: | no |
| Tester: | Unknown | Verified working: | no |
| Review URL(s): | |||
Description
Background
We're having collecting of weak references mechanism in Disposer (https://hg.adblockplus.org/libadblockplus-android/file/tip/libadblockplus-android/src/org/adblockplus/libadblockplus/Disposer.java). So they are GCed eventually though we don't know the moment exactly.
But unfortunately from C++ point of view we have to dispose all JsValues that keeping references to JsEngine/FilterEngine have to be disposed BEFORe disposing of JsEngine/FilterEngine. So we can invoke dispose explicitly just like we do for logSystem in AdblockEngine.dispose(). We can count references count while releasing JsEngine/FilterEngine smart pointer.
It's unsafe to leave it as-is (as any leaked JsValue instance will keep a reference to JsEngine/FilterEngine and prevents them from being released and they continue to work and use released callbacks from JNI which leads to crash like #5026) and we should return back to this later.
What to change
Invoke dispose for any objects which classes inherit from JsValue asap.
Attachments (0)
Change History (4)
comment:1 Changed on 03/29/2017 at 12:14:48 PM by asmirnov
- Review URL(s) modified (diff)
- Status changed from new to reviewing
comment:2 Changed on 03/29/2017 at 12:37:02 PM by sergz
- Blocking 5026 added
comment:3 Changed on 03/30/2017 at 07:27:25 PM by abpbot
comment:4 Changed on 03/30/2017 at 07:28:31 PM by asmirnov
- Resolution set to fixed
- Status changed from reviewing to closed
A commit referencing this issue has landed:
Issue 5053 - Release JsValues explicitly