JsEngine crashes in dtor with LogSystemPtr ref

[asmirnov]

Environment

The issue is related to #5031. If passing logSystem reference as new AdblockPlus::LogSystemPtr(new JniLogSystemCallback(env, callbackObject)) as approved in code review to #5031 JsEngine crashes in dtor. LogSystem is disposed as previously (logSystem.dispose()).

BTW if logSystem ptr is passed as before #5031 no crash is observed.
Should #5031 be reverted or anything in JsEngine fixed?

How to reproduce

1. run the app
2. close the app
3. make sure it's crashed

...

pid: 10813, tid: 10813, name: roid.webviewapp  >>> org.adblockplus.libadblockplus.android.webviewapp <<<
03-31 01:11:08.203 1169-1169/? I/DEBUG: signal 11 (SIGSEGV), code 1 (SEGV_MAPERR), fault addr 0x83002ba0
03-31 01:11:08.212 1169-1169/? I/DEBUG:     eax a28ea1a0  ebx a25f6d30  ecx 81000054  edx b43be830
03-31 01:11:08.213 1169-1169/? I/DEBUG:     esi a2890c20  edi ffffffff
03-31 01:11:08.214 1169-1169/? I/DEBUG:     xcs 00000073  xds 0000007b  xes 0000007b  xfs 00000007  xss 0000007b
03-31 01:11:08.214 1169-1169/? I/DEBUG:     eip 6f6c6273  ebp bfd634f8  esp bfd634ac  flags 00210202
03-31 01:11:08.216 1169-1169/? I/DEBUG: backtrace:
03-31 01:11:08.216 1169-1169/? I/DEBUG:     #00 pc 0022e273  /data/dalvik-cache/x86/system@framework@boot.art
03-31 01:11:08.216 1169-1169/? I/DEBUG:     #01 pc 001be59d  /data/app/org.adblockplus.libadblockplus.android.webviewapp-1/lib/x86/libadblockplus-jni.so (AdblockPlus::JsEngine::~JsEngine()+253)
03-31 01:11:08.216 1169-1169/? I/DEBUG:     #02 pc 001be6eb  /data/app/org.adblockplus.libadblockplus.android.webviewapp-1/lib/x86/libadblockplus-jni.so (std::__1::__shared_ptr_pointer<AdblockPlus::JsEngine*, std::__1::default_delete<AdblockPlus::JsEngine>, std::__1::allocator<AdblockPlus::JsEngine> >::__on_zero_shared()+43)
03-31 01:11:08.216 1169-1169/? I/DEBUG:     #03 pc 0051fdf5  /data/app/org.adblockplus.libadblockplus.android.webviewapp-1/lib/x86/libadblockplus-jni.so
03-31 01:11:08.218 1169-1169/? I/DEBUG:     #04 pc 001a17e7  /data/app/org.adblockplus.libadblockplus.android.webviewapp-1/lib/x86/libadblockplus-jni.so
03-31 01:11:08.218 1169-1169/? I/DEBUG:     #05 pc 00173ad2  /data/dalvik-cache/x86/data@app@org.adblockplus.libadblockplus.android.webviewapp-1@base.apk@classes.dex
03-31 01:11:08.218 1169-1169/? I/DEBUG:     #06 pc 00090acf  [anon:libc_malloc]
03-31 01:11:08.218 1169-1169/? I/DEBUG:     #07 pc 00090acf  [anon:libc_malloc]
03-31 01:11:08.301 1169-1169/? I/DEBUG: Tombstone written to: /data/tombstones/tombstone_02

Observed behaviour

crash after the app is closed

Expected behaviour

nothing crashes

[asmirnov]

BTW disposals count is equal to allocations count and JsEngine refs count is 1 on it's disposal (so everything seems to be ok)

[sergz]

I think it's because of the line ​https://github.com/adblockplus/libadblockplus-android/blob/f877b6f712c98e60059f70ac848fef1684b5c6bf/libadblockplus-android/jni/JniJsEngine.cpp#L171. it should be AdblockPlus::LogSystemPtr logSystem = *JniLongToTypePtr<AdblockPlus::LogSystemPtr>(logSystemPtr);
We convert std::shared_ptr<LogSystem>* to int and then convert that int to LogSystem*, that's wrong.
Currently destructor of JsEngine is calling destructor of its memeber std::shared_ptr<LogSystem> which is trying to delete stored pointer as LogSystem (call LogSystem::~LogSystem), but the stored pointer points to std::shared_ptr<LogSystem>.

[abpbot]

A commit referencing this issue has landed:
​Issue 5075 - JsEngine crashes in dtor with LogSystemPtr ref