Opened 3 years ago

Closed 3 years ago

#5016 closed defect (duplicate)

Crash with no UpdateCheckDoneCallback passed

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

Description

Environment

We have to force update subscriptions right after preloaded resource is returned. So we invoke filterEngine.forceUpdateCheck() in java code and it invokes engine->forceUpdateCheck(0):
https://hg.adblockplus.org/libadblockplus-android/file/tip/libadblockplus-android/jni/JniFilterEngine.cpp#l246

When the callback is about to be invoked the app crashes with signal 6 SIGABRT.
I think it tries to invoke a callback using null pointer.

How to reproduce

  1. invoke filterEngine.forceUpdateCheck()
  2. wait for few seconds
  3. make sure the app crashed

...

Observed behaviour

the app crashes

Expected behaviour

check is done with no any callback fired

---

I've modified c+ jni class to output ptr value passed:

static void JNICALL JniForceUpdateCheck(JNIEnv* env, jclass clazz, jlong ptr, jlong updaterPtr)
{
  AdblockPlus::FilterEngine* engine = JniLongToTypePtr<AdblockPlus::FilterEngine>(ptr);
  JniUpdateCheckDoneCallback* callback =
      JniLongToTypePtr<JniUpdateCheckDoneCallback>(updaterPtr);

  AdblockPlus::FilterEngine::UpdateCheckDoneCallback
      updateCheckDoneCallback = 0;

  __android_log_print(ANDROID_LOG_DEBUG, "abp_ndk", "updatePtr = %d", (int)updaterPtr);

  if (updaterPtr)
  {
    updateCheckDoneCallback =
        std::bind(&JniUpdateCheckDoneCallback::Callback, callback,
                       std::placeholders::_1);
  }

  try
  {
    engine->ForceUpdateCheck(updateCheckDoneCallback);
  }
  CATCH_AND_THROW(env)
}

and you can see 0 is passed:

03-20 12:46:51.535 4848-4848/org.adblockplus.libadblockplus.android.webviewapp D/abp_ndk: updatePtr = 0

stacktrace:

03-20 12:46:56.201 5169-5169/? A/DEBUG: signal 6 (SIGABRT), code -6 (SI_TKILL), fault addr --------
03-20 12:46:56.201 5169-5169/? A/DEBUG:     r0 00000000  r1 000013cd  r2 00000006  r3 00000008
03-20 12:46:56.201 5169-5169/? A/DEBUG:     r4 d0683978  r5 00000006  r6 d0683920  r7 0000010c
03-20 12:46:56.201 5169-5169/? A/DEBUG:     r8 eb85eb60  r9 d0682ec4  sl d5734711  fp d39e7040
03-20 12:46:56.201 5169-5169/? A/DEBUG:     ip 00000000  sp d0682970  lr f1a5b497  pc f1a5dd18  cpsr 200b0010
03-20 12:46:56.210 5169-5169/? A/DEBUG: backtrace:
03-20 12:46:56.210 5169-5169/? A/DEBUG:     #00 pc 00049d18  /system/lib/libc.so (tgkill+12)
03-20 12:46:56.210 5169-5169/? A/DEBUG:     #01 pc 00047493  /system/lib/libc.so (pthread_kill+34)
03-20 12:46:56.210 5169-5169/? A/DEBUG:     #02 pc 0001d769  /system/lib/libc.so (raise+10)
03-20 12:46:56.211 5169-5169/? A/DEBUG:     #03 pc 00019261  /system/lib/libc.so (__libc_android_abort+34)
03-20 12:46:56.211 5169-5169/? A/DEBUG:     #04 pc 000172c8  /system/lib/libc.so (abort+4)
03-20 12:46:56.211 5169-5169/? A/DEBUG:     #05 pc 0049819d  /data/app/org.adblockplus.libadblockplus.android.webviewapp-1/lib/arm/libadblockplus-jni.so
03-20 12:46:56.211 5169-5169/? A/DEBUG:     #06 pc 00498269  /data/app/org.adblockplus.libadblockplus.android.webviewapp-1/lib/arm/libadblockplus-jni.so
03-20 12:46:56.211 5169-5169/? A/DEBUG:     #07 pc 0048d0c7  /data/app/org.adblockplus.libadblockplus.android.webviewapp-1/lib/arm/libadblockplus-jni.so
03-20 12:46:56.211 5169-5169/? A/DEBUG:     #08 pc 0048c777  /data/app/org.adblockplus.libadblockplus.android.webviewapp-1/lib/arm/libadblockplus-jni.so (__cxa_throw+182)
03-20 12:46:56.211 5169-5169/? A/DEBUG:     #09 pc 001af54d  /data/app/org.adblockplus.libadblockplus.android.webviewapp-1/lib/arm/libadblockplus-jni.so (_ZN11AdblockPlus12FilterEngine15UpdateCheckDoneERKNSt3__112basic_stringIcNS1_11char_traitsIcEENS1_9allocatorIcEEEENS1_8functionIFvS9_EEERNS1_6vectorINS1_10shared_ptrINS_7JsValueEEENS5_ISG_EEEE+120)
03-20 12:46:56.211 5169-5169/? A/DEBUG:     #10 pc 001affb5  /data/app/org.adblockplus.libadblockplus.android.webviewapp-1/lib/arm/libadblockplus-jni.so (_ZNSt3__110__function6__funcINS_6__bindIMN11AdblockPlus12FilterEngineEFvRKNS_12basic_stringIcNS_11char_traitsIcEENS_9allocatorIcEEEENS_8functionIFvSC_EEERNS_6vectorINS_10shared_ptrINS3_7JsValueEEENS8_ISJ_EEEEEJPS4_RSA_RSF_RNS_12placeholders4__phILi1EEEEEENS8_ISW_EEFvSM_EEclESM_+96)
03-20 12:46:56.211 5169-5169/? A/DEBUG:     #11 pc 001b14eb  /data/app/org.adblockplus.libadblockplus.android.webviewapp-1/lib/arm/libadblockplus-jni.so (_ZN11AdblockPlus8JsEngine12TriggerEventERKNSt3__112basic_stringIcNS1_11char_traitsIcEENS1_9allocatorIcEEEERNS1_6vectorINS1_10shared_ptrINS_7JsValueEEENS5_ISD_EEEE+34)
03-20 12:46:56.211 5169-5169/? A/DEBUG:     #12 pc 001b67f3  /data/app/org.adblockplus.libadblockplus.android.webviewapp-1/lib/arm/libadblockplus-jni.so
03-20 12:46:56.211 5169-5169/? A/DEBUG:     #13 pc 001e1694  /data/app/org.adblockplus.libadblockplus.android.webviewapp-1/lib/arm/libadblockplus-jni.so (_ZN2v88internal25FunctionCallbackArguments4CallEPFNS_6HandleINS_5ValueEEERKNS_9ArgumentsEE+368)
03-20 12:46:56.211 5169-5169/? A/DEBUG:     #14 pc 001ef8d0  /data/app/org.adblockplus.libadblockplus.android.webviewapp-1/lib/arm/libadblockplus-jni.so
03-20 12:46:56.211 5169-5169/? A/DEBUG:     #15 pc 00000b78  <anonymous:20a0a000>

Attachments (1)

callback.txt (632.2 KB) - added by asmirnov 3 years ago.

Download all attachments as: .zip

Change History (9)

Changed 3 years ago by asmirnov

comment:1 Changed 3 years ago by asmirnov

  • Ready set

comment:2 Changed 3 years ago by asmirnov

  • Blocking 5010 added

comment:3 Changed 3 years ago by asmirnov

  • Review URL(s) modified (diff)
  • Status changed from new to reviewing

comment:4 Changed 3 years ago by asmirnov

  • Review URL(s) modified (diff)
  • Status changed from reviewing to reopened

comment:5 Changed 3 years ago by sergz

  • Cc sergz added
  • Owner set to sergz

Yes, it tries to call an empty callback. I would not say that it's a bug if the parameter were not marked as optional.

I am going to add the check whether the callback is not an empty function before calling of the it.

comment:6 Changed 3 years ago by asmirnov

Yes, i've been thinking about it but i rather expect to see that the argument is required than not so see that the argument is optional.
A question of agreement though.

I can see https://hg.adblockplus.org/libadblockplus-android/file/tip/libadblockplus-android/src/org/adblockplus/libadblockplus/FilterEngine.java#l95 method with no argument so i assume it's expected use case.
That's why i decided that the argument of c++ class is meant to be optional and this is actually a bug and not just required argument.

comment:7 Changed 3 years ago by asmirnov

BTW i've checked with not null callback and it's fired after check is done.
So with not null callback it's working as expected.

comment:8 Changed 3 years ago by sergz

  • Resolution set to duplicate
  • Status changed from reopened to closed

There is already ticket #1397.

Note: See TracTickets for help on using tickets.