Opened on 03/22/2017 at 06:29:06 AM

Closed on 03/23/2018 at 10:33:09 AM

#5026 closed defect (fixed)

Libadblockplus crashes while calling some callback after dispose

Reported by: asmirnov Assignee:
Priority: P2 Milestone:
Module: Libadblockplus Keywords:
Cc: Blocked By: #5030, #5031, #5053
Blocking: Platform: Unknown / Cross platform
Ready: yes Confidential: no
Tester: Unknown Verified working: no
Review URL(s):

Description (last modified by asmirnov)

Environment

Libadblockplus and core probably creates some background threads and uses callbacks from java/android wrapper. After android app is finished FilterEngine::dispose() is invoked and c++ filter engine instance is disposed too.

How to reproduce

  1. start android app
  2. navigate to some URL (e1.ru in my case)
  3. press back to exit the app
  4. wait for few seconds/minutes
  5. make sure you see "unfortunately ... app has stopped " android alert and crash stacktrace in android log

...

Observed behaviour

app crashes with sigsegv (signal 11)

Expected behaviour

nothing (the app just exists)

I'm testing https://codereview.adblockplus.org/29379647/ with https://codereview.adblockplus.org/29391555/ binaries.

I've modified JNI layer (https://codereview.adblockplus.org/29379647/diff/29390630/libadblockplus-android/jni/JniFilterEngine.cpp) to see the pointers as following:

static jlong JNICALL JniCtor(JNIEnv* env, jclass clazz, jlong jsEnginePtr, jlong isAllowedConnectionCallbackPtr)
{
  try
  {
    AdblockPlus::JsEnginePtr& jsEngine = *JniLongToTypePtr<AdblockPlus::JsEnginePtr>(jsEnginePtr);
    AdblockPlus::FilterEnginePtr* filterEngine = NULL;

    if (isAllowedConnectionCallbackPtr != 0)
    {
      AdblockPlus::FilterEngine::CreationParameters creationParameters;
      JniIsAllowedConnectionTypeCallback* callback =
        JniLongToTypePtr<JniIsAllowedConnectionTypeCallback>(isAllowedConnectionCallbackPtr);

      AdblockPlus::FilterEngine::IsConnectionAllowedCallback cppCallback =
        std::bind(&JniIsAllowedConnectionTypeCallback::Callback, callback, std::placeholders::_1);
      creationParameters.isConnectionAllowedCallback = cppCallback;

      filterEngine = new AdblockPlus::FilterEnginePtr(
        AdblockPlus::FilterEngine::Create(jsEngine, creationParameters));
    }
    else
    {
      filterEngine = new AdblockPlus::FilterEnginePtr(
        AdblockPlus::FilterEngine::Create(jsEngine));
    }

    __android_log_print(ANDROID_LOG_DEBUG, APPNAME, "Created FilterEngine %p (%p)", filterEngine, filterEngine->get());
    return JniPtrToLong(filterEngine);
  }
  CATCH_THROW_AND_RETURN(env, 0)
}
static void JNICALL JniDtor(JNIEnv* env, jclass clazz, jlong ptr)
{
  AdblockPlus::FilterEnginePtr* engine = JniLongToTypePtr<AdblockPlus::FilterEnginePtr>(ptr);
  __android_log_print(ANDROID_LOG_DEBUG, APPNAME, "Disposing FilterEngine %p (%p)", engine, engine->get());
  delete engine;
}

i can see in the log filter engine is created and disposed:

D/libabp-android_JNI: Created FilterEngine 0xae85a9f0 (0xae839e40)
...
D/libabp-android_JNI: Disposing FilterEngine 0xae85a9f0 (0xae839e40)

i think dispose should release resources and update threads callbacks correctly or even signal the threads to exit.

stacktrace (see attached log file for more info):

03-22 11:17:21.597 1169-1169/? I/DEBUG: signal 11 (SIGSEGV), code 1 (SEGV_MAPERR), fault addr 0x4f1bf4c1
03-22 11:17:21.609 1169-1169/? I/DEBUG:     eax ae9a337e  ebx a24f2d31  ecx 00000039  edx ffffffff
03-22 11:17:21.610 1169-1169/? I/DEBUG:     esi 9e2fef30  edi 9e2fef40
03-22 11:17:21.610 1169-1169/? I/DEBUG:     xcs 00000073  xds 0000007b  xes 0000007b  xfs 000000b7  xss 0000007b
03-22 11:17:21.610 1169-1169/? I/DEBUG:     eip aea9a015  ebp 9e2ff138  esp 9e2feeec  flags 00210282
03-22 11:17:21.610 1169-1169/? I/DEBUG: backtrace:
03-22 11:17:21.610 1169-1169/? I/DEBUG:     #00 pc 0029a015  [anon:libc_malloc]
03-22 11:17:21.610 1169-1169/? I/DEBUG:     #01 pc 001c677b  /data/app/org.adblockplus.libadblockplus.android.webviewapp-1/lib/x86/libadblockplus-jni.so ((anonymous namespace)::ErrorCallback(v8::Arguments const&)+43)
03-22 11:17:21.610 1169-1169/? I/DEBUG:     #02 pc 001f6ed8  /data/app/org.adblockplus.libadblockplus.android.webviewapp-1/lib/x86/libadblockplus-jni.so (v8::internal::FunctionCallbackArguments::Call(v8::Handle<v8::Value> (*)(v8::Arguments const&))+296)
03-22 11:17:21.610 1169-1169/? I/DEBUG:     #03 pc 00207bcd  /data/app/org.adblockplus.libadblockplus.android.webviewapp-1/lib/x86/libadblockplus-jni.so (v8::internal::Builtin_HandleApiCall(int, v8::internal::Object**, v8::internal::Isolate*)+317)
03-22 11:17:21.610 1169-1169/? I/DEBUG:     #04 pc 00000895  <unknown>
03-22 11:17:21.610 1169-1169/? I/DEBUG:     #05 pc 000f3d4c  <unknown>
03-22 11:17:21.610 1169-1169/? I/DEBUG:     #06 pc 00054cda  <unknown>
03-22 11:17:21.610 1169-1169/? I/DEBUG:     #07 pc 0001c5d8  <unknown>
03-22 11:17:21.610 1169-1169/? I/DEBUG:     #08 pc 0000e449  <unknown>
03-22 11:17:21.610 1169-1169/? I/DEBUG:     #09 pc 002400fd  /data/app/org.adblockplus.libadblockplus.android.webviewapp-1/lib/x86/libadblockplus-jni.so (v8::internal::Invoke(bool, v8::internal::Handle<v8::internal::JSFunction>, v8::internal::Handle<v8::internal::Object>, int, v8::internal::Handle<v8::internal::Object>*, bool*)+253)
03-22 11:17:21.610 1169-1169/? I/DEBUG:     #10 pc 00241866  /data/app/org.adblockplus.libadblockplus.android.webviewapp-1/lib/x86/libadblockplus-jni.so (v8::internal::Execution::Call(v8::internal::Handle<v8::internal::Object>, v8::internal::Handle<v8::internal::Object>, int, v8::internal::Handle<v8::internal::Object>*, bool*, bool)+246)
03-22 11:17:21.610 1169-1169/? I/DEBUG:     #11 pc 003a9fc2  /data/app/org.adblockplus.libadblockplus.android.webviewapp-1/lib/x86/libadblockplus-jni.so (v8::internal::Runtime_Apply(int, v8::internal::Object**, v8::internal::Isolate*)+482)
03-22 11:17:21.610 1169-1169/? I/DEBUG:     #12 pc 00000895  <unknown>
03-22 11:17:21.610 1169-1169/? I/DEBUG:     #13 pc 0004f664  <unknown>
03-22 11:17:21.610 1169-1169/? I/DEBUG:     #14 pc 00008900  <unknown>
03-22 11:17:21.610 1169-1169/? I/DEBUG:     #15 pc 000553b0  <unknown>
03-22 11:17:21.610 1169-1169/? I/DEBUG:     #16 pc 0001c5d8  <unknown>
03-22 11:17:21.610 1169-1169/? I/DEBUG:     #17 pc 0000e449  <unknown>
03-22 11:17:21.610 1169-1169/? I/DEBUG:     #18 pc 002400fd  /data/app/org.adblockplus.libadblockplus.android.webviewapp-1/lib/x86/libadblockplus-jni.so (v8::internal::Invoke(bool, v8::internal::Handle<v8::internal::JSFunction>, v8::internal::Handle<v8::internal::Object>, int, v8::internal::Handle<v8::internal::Object>*, bool*)+253)
03-22 11:17:21.610 1169-1169/? I/DEBUG:     #19 pc 00241866  /data/app/org.adblockplus.libadblockplus.android.webviewapp-1/lib/x86/libadblockplus-jni.so (v8::internal::Execution::Call(v8::internal::Handle<v8::internal::Object>, v8::internal::Handle<v8::internal::Object>, int, v8::internal::Handle<v8::internal::Object>*, bool*, bool)+246)
03-22 11:17:21.610 1169-1169/? I/DEBUG:     #20 pc 003a9fc2  /data/app/org.adblockplus.libadblockplus.android.webviewapp-1/lib/x86/libadblockplus-jni.so (v8::internal::Runtime_Apply(int, v8::internal::Object**, v8::internal::Isolate*)+482)
03-22 11:17:21.610 1169-1169/? I/DEBUG:     #21 pc 00000895  <unknown>
03-22 11:17:21.610 1169-1169/? I/DEBUG:     #22 pc 0004f664  <unknown>
03-22 11:17:21.610 1169-1169/? I/DEBUG:     #23 pc 00008900  <unknown>
03-22 11:17:21.610 1169-1169/? I/DEBUG:     #24 pc 00055ef8  <unknown>
03-22 11:17:21.610 1169-1169/? I/DEBUG:     #25 pc 0001c5d8  <unknown>
03-22 11:17:21.610 1169-1169/? I/DEBUG:     #26 pc 0000e449  <unknown>
03-22 11:17:21.610 1169-1169/? I/DEBUG:     #27 pc 002400fd  /data/app/org.adblockplus.libadblockplus.android.webviewapp-1/lib/x86/libadblockplus-jni.so (v8::internal::Invoke(bool, v8::internal::Handle<v8::internal::JSFunction>, v8::internal::Handle<v8::internal::Object>, int, v8::internal::Handle<v8::internal::Object>*, bool*)+253)
03-22 11:17:21.610 1169-1169/? I/DEBUG:     #28 pc 00241866  /data/app/org.adblockplus.libadblockplus.android.webviewapp-1/lib/x86/libadblockplus-jni.so (v8::internal::Execution::Call(v8::internal::Handle<v8::internal::Object>, v8::internal::Handle<v8::internal::Object>, int, v8::internal::Handle<v8::internal::Object>*, bool*, bool)+246)
03-22 11:17:21.610 1169-1169/? I/DEBUG:     #29 pc 003a9fc2  /data/app/org.adblockplus.libadblockplus.android.webviewapp-1/lib/x86/libadblockplus-jni.so (v8::internal::Runtime_Apply(int, v8::internal::Object**, v8::internal::Isolate*)+482)
03-22 11:17:21.610 1169-1169/? I/DEBUG:     #30 pc 00000895  <unknown>
03-22 11:17:21.611 1169-1169/? I/DEBUG:     #31 pc 0004f664  <unknown>
03-22 11:17:21.611 1169-1169/? I/DEBUG:     #32 pc 00008900  <unknown>
03-22 11:17:21.611 1169-1169/? I/DEBUG:     #33 pc 0001c5d1  <unknown>
03-22 11:17:21.611 1169-1169/? I/DEBUG:     #34 pc 0000e449  <unknown>
03-22 11:17:21.611 1169-1169/? I/DEBUG:     #35 pc 002400fd  /data/app/org.adblockplus.libadblockplus.android.webviewapp-1/lib/x86/libadblockplus-jni.so (v8::internal::Invoke(bool, v8::internal::Handle<v8::internal::JSFunction>, v8::internal::Handle<v8::internal::Object>, int, v8::internal::Handle<v8::internal::Object>*, bool*)+253)
03-22 11:17:21.611 1169-1169/? I/DEBUG:     #36 pc 00241866  /data/app/org.adblockplus.libadblockplus.android.webviewapp-1/lib/x86/libadblockplus-jni.so (v8::internal::Execution::Call(v8::internal::Handle<v8::internal::Object>, v8::internal::Handle<v8::internal::Object>, int, v8::internal::Handle<v8::internal::Object>*, bool*, bool)+246)
03-22 11:17:21.611 1169-1169/? I/DEBUG:     #37 pc 001e0bca  /data/app/org.adblockplus.libadblockplus.android.webviewapp-1/lib/x86/libadblockplus-jni.so (v8::Function::Call(v8::Handle<v8::Object>, int, v8::Handle<v8::Value>*)+330)
03-22 11:17:21.611 1169-1169/? I/DEBUG:     #38 pc 001c16e4  /data/app/org.adblockplus.libadblockplus.android.webviewapp-1/lib/x86/libadblockplus-jni.so (AdblockPlus::JsValue::Call(std::__1::vector<std::__1::shared_ptr<AdblockPlus::JsValue>, std::__1::allocator<std::__1::shared_ptr<AdblockPlus::JsValue> > > const&, std::__1::shared_ptr<AdblockPlus::JsValue>) const+740)
03-22 11:17:21.611 1169-1169/? I/DEBUG:     #39 pc 001c59a5  /data/app/org.adblockplus.libadblockplus.android.webviewapp-1/lib/x86/libadblockplus-jni.so ((anonymous namespace)::WebRequestThread::Run()+965)
03-22 11:17:21.611 1169-1169/? I/DEBUG:     #40 pc 001c47a4  /data/app/org.adblockplus.libadblockplus.android.webviewapp-1/lib/x86/libadblockplus-jni.so (AdblockPlus::Thread::CallRun(AdblockPlus::Thread*)+36)
03-22 11:17:21.611 1169-1169/? I/DEBUG:     #41 pc 000211a8  /system/lib/libc.so (__pthread_start(void*)+56)
03-22 11:17:21.611 1169-1169/? I/DEBUG:     #42 pc 0001c529  /system/lib/libc.so (__start_thread+25)
03-22 11:17:21.611 1169-1169/? I/DEBUG:     #43 pc 000130f6  /system/lib/libc.so (__bionic_clone+70)

the native libraries (.so) are not stripped though you can see "<unknown>" in stack trace.

Attachments (2)

callback_crash.log (159.9 KB) - added by asmirnov on 03/22/2017 at 06:29:18 AM.
callback_crash_arm.log.zip (89.3 KB) - added by asmirnov on 03/22/2017 at 06:44:45 AM.
on ARM

Download all attachments as: .zip

Change History (13)

Changed on 03/22/2017 at 06:29:18 AM by asmirnov

comment:1 Changed on 03/22/2017 at 06:33:28 AM by asmirnov

  • Description modified (diff)

comment:2 Changed on 03/22/2017 at 06:44:04 AM by asmirnov

BTW i've been testing on emulator (x86), just reproduced on device (zte blade v8, arm)

Changed on 03/22/2017 at 06:44:45 AM by asmirnov

on ARM

comment:3 Changed on 03/22/2017 at 06:54:02 AM by asmirnov

According to ARM log it seems to be log callback usage after disposing:

_ZN20JniLogSystemCallbackclEN11AdblockPlus9LogSystem8LogLevelERKNSt3112basic_stringIcNS3_11char_traitsIcEENS3_9allocatorIcEEEESB_

comment:4 Changed on 03/22/2017 at 08:31:11 AM by sergz

I would say it's a duplicate of #4692 and #3595.

comment:5 Changed on 03/22/2017 at 12:08:59 PM by asmirnov

i've checked JsEngine smart pointer to be released (https://hg.adblockplus.org/libadblockplus-android/file/tip/libadblockplus-android/src/org/adblockplus/libadblockplus/android/AdblockEngine.java#l220) too:

static jlong JNICALL JniCtor(JNIEnv* env, jclass clazz, jobject jAppInfo)
{
  AdblockPlus::AppInfo appInfo;

  TransformAppInfo(env, jAppInfo, appInfo);

  try
  {
    AdblockPlus::JsEnginePtr* jsEngine = new AdblockPlus::JsEnginePtr(AdblockPlus::JsEngine::New(appInfo));
    __android_log_print(ANDROID_LOG_DEBUG, APPNAME, "Created JsEngine %p (%p)", jsEngine, jsEngine->get());
    return JniPtrToLong(jsEngine);
  }
  CATCH_THROW_AND_RETURN(env, 0)
}
static void JNICALL JniDtor(JNIEnv* env, jclass clazz, jlong ptr)
{
  AdblockPlus::JsEnginePtr* engine = JniLongToTypePtr<AdblockPlus::JsEnginePtr>(ptr);
  __android_log_print(ANDROID_LOG_DEBUG, APPNAME, "Disposing JsEngine %p (%p)", engine, engine->get());
  delete engine;
}

log:

03-22 17:06:33.799 2871-2909/org.adblockplus.libadblockplus.android.webviewapp D/libabp-android_JNI: Created JsEngine 0xae88a818 (0xae953700)
03-22 17:06:34.919 2871-2909/org.adblockplus.libadblockplus.android.webviewapp D/libabp-android_JNI: Created FilterEngine 0xae88ab30 (0xae839e90)
03-22 17:07:01.732 2871-2871/org.adblockplus.libadblockplus.android.webviewapp D/libabp-android_JNI: Disposing FilterEngine 0xae88ab30 (0xae839e90)
03-22 17:07:01.732 2871-2871/org.adblockplus.libadblockplus.android.webviewapp D/libabp-android_JNI: Disposing JsEngine 0xae88a818 (0xae953700)

comment:6 Changed on 03/22/2017 at 12:36:46 PM by asmirnov

yes, it was usage of dispose log callback.

I commented out disposing of logSystem https://hg.adblockplus.org/libadblockplus-android/file/tip/libadblockplus-android/src/org/adblockplus/libadblockplus/android/AdblockEngine.java#l162 . (and created memory leak for not disposed logSystem) and got error message:

03-22 17:22:17.632 15797-16034/org.adblockplus.libadblockplus.android.webviewapp E/libabp-android_JNIcompat.js:79: Adblock Plus: Downloading URL https://notification.adblockplus.org/notification.json failed (synchronize_connection_error)
                                                                                                                   Download address: https://notification.adblockplus.org/notification.json?addonName=adblockplusandroid&addonVersion=1.0.1&application=android&applicationVersion=24&platform=libadblockplus&platformVersion=1.0&lastVersion=0&downloadCount=0
                                                                                                                   Channel status: 2152398861
                                                                                                                   Server response: 0
03-22 17:22:17.636 15797-16037/org.adblockplus.libadblockplus.android.webviewapp E/libabp-android_JNIcompat.js:79: Adblock Plus: Downloading URL https://easylist-downloads.adblockplus.org/exceptionrules.txt failed (synchronize_connection_error)
                                                                                                                   Download address: https://easylist-downloads.adblockplus.org/exceptionrules.txt?addonName=adblockplusandroid&addonVersion=1.0.1&application=android&applicationVersion=24&platform=libadblockplus&platformVersion=1.0&lastVersion=201703210741&downloadCount=1
                                                                                                                   Channel status: 2152398861
                                                                                                                   Server response: 0

so JsEngine still continues to get notifications after it's disposed (JniJsEngine), fails, tries to report about the failure to disposed log callback and crashes.

comment:7 Changed on 03/22/2017 at 04:56:04 PM by sergz

  • Blocked By 5030 added

comment:8 Changed on 03/22/2017 at 05:06:39 PM by sergz

  • Blocked By 5031 added

comment:9 Changed on 03/28/2017 at 08:03:17 AM by asmirnov

  • Blocking 4948 removed

comment:10 Changed on 03/29/2017 at 12:37:02 PM by sergz

  • Blocked By 5053 added

comment:11 Changed on 03/23/2018 at 10:33:09 AM by sergz

  • Resolution set to fixed
  • Status changed from new to closed

Since #5179 is landed there should not be any thread using FilterEngine, JsEngine or whatever else after destroying of the implementation of Platform provided by default.

Add Comment

Modify Ticket

Change Properties
Action
as closed .
The resolution will be deleted. Next status will be 'reopened'.
to The owner will be changed from (none).
 
Note: See TracTickets for help on using tickets.