google / oboe

Oboe is a C++ library that makes it easy to build high-performance audio apps on Android.
Apache License 2.0
3.71k stars 569 forks source link

oboe crash in android::CallbackProtector::enterCbIfOk(android::sp<android::CallbackProtector> const&) #2032

Open lhran99 opened 4 months ago

lhran99 commented 4 months ago

Android version(s): Android 10,level 29 Oboe version:1.6.1

We found that customers occasionally reported some oboe crashes, but we could only get the stack trace and no other information. Here is the backtrace.


Crash type: 'native' Start time: '2024-05-31T16:49:23.021+0800' Crash time: '2024-05-31T17:02:35.474+0800' App version: '9.0.65.6558' Rooted: 'Yes' API level: '29' Build fingerprint: 'WeTestC2/WeTestC2/WeTestC2:10/master_20210324/mp1V6:userdebug/test-keys' ABI: 'arm64' pid: 15987, tid: 6951, name: AudioRecord >>> com.tencent.mobileqq <<< signal 11 (SIGSEGV), code 1 (SEGV_MAPERR), fault addr 0x55555555555565 x0 5555555555555565 x1 00000068ffc43800 x2 0000000000000000 x3 0000000000000000 x4 0000000000000000 x5 0000000000000000 x6 0000000000000000 x7 7020726f6620676e x8 0000000000000001 x9 00000000003ac698 x10 0000000000000002 x11 0000000000000000 x12 000000758a73b700 x13 786574756d206461 x14 00000000ffffffff x15 0000000000000000 x16 000000758d33e000 x17 000000758d675e48 x18 0000006938efe000 x19 5555555555555565 x20 5555555555555555 x21 0000000000000000 x22 0000000000000004 x23 0000000000000000 x24 0000000000000000 x25 000000758d31f508 x26 000000758a72e000 x27 0000000000000000 x28 0000007410327020 x29 0000007410326a90 sp 0000007410326a70 lr 000000758d321904 pc 000000758d675e48

backtrace:

00 pc 00000000000e2e48 /apex/com.android.runtime/lib64/bionic/libc.so (pthread_mutex_lock)

#01 pc 0000000000020900  /system/lib64/libwilhelm.so (_ZN7android17CallbackProtector11enterCbIfOkERKNS_2spIS0_EE+32)
#02 pc 000000000001e534  /system/lib64/libwilhelm.so (_ZL22audioRecorder_callbackiPvS_+44)
#03 pc 0000000000051e08  /system/lib64/libaudioclient.so (_ZN7android11AudioRecord18processAudioBufferEv+1184)
#04 pc 0000000000051670  /system/lib64/libaudioclient.so (_ZN7android11AudioRecord17AudioRecordThread10threadLoopEv+264)
#05 pc 00000000000137c0  /system/lib64/libutils.so (_ZN7android6Thread11_threadLoopEPv+312)
#06 pc 00000000000c3814  /system/lib64/libandroid_runtime.so (_ZN7android14AndroidRuntime15javaThreadShellEPv+140)
#07 pc 00000000000e2364  /apex/com.android.runtime/lib64/bionic/libc.so (_ZL15__pthread_startPv+36)
#08 pc 0000000000083d98  /apex/com.android.runtime/lib64/bionic/libc.so (__start_thread+64)

build id: /apex/com.android.runtime/lib64/bionic/libc.so (BuildId: 2a5abdc9c768b33656f7aa8d9ce5cf54. FileSize: 1235048. LastModified: 2009-01-01T08:00:00.000+0800. MD5: 6d657f068fa7bd8fd404b0a773f10858) /system/lib64/libwilhelm.so (BuildId: 7cf76b1ab2372a467f810635989d3f0a. FileSize: 264408. LastModified: 2009-01-01T08:00:00.000+0800. MD5: 9939f861fcf4c29c00cd853b588ca8f7) /system/lib64/libaudioclient.so (BuildId: 1fa614fdcf3e7894395e5a0e1dfda7db. FileSize: 750008. LastModified: 2009-01-01T08:00:00.000+0800. MD5: 6d8f5a25349a1d17664f55a42a5baeb0) /system/lib64/libutils.so (BuildId: f013cbada038685234657c61db8227dc. FileSize: 117416. LastModified: 2009-01-01T08:00:00.000+0800. MD5: 262abf816fc16b435fbf71b14b2e55a9) /system/lib64/libandroid_runtime.so (BuildId: 7e1b18b9556aaae0eced3eee5ae9d89e. FileSize: 2108280. LastModified: 2009-01-01T08:00:00.000+0800. MD5: a379bfea2c3f1c11f60f7aa1abe065c2)

I try to analysis it and here is the infomation:

  1. the frame#01 code is as follows:

    bool CallbackProtector::enterCbIfOk(const sp<CallbackProtector> &protector) {
      if (protector != 0) {
          return protector->enterCb();
      } else {
          SL_LOGE("Callback protector is missing");
          return false;
      }
    }
    
    bool CallbackProtector::enterCb() {
      Mutex::Autolock _l(mLock); // it seems protector is invalid,so get the invalid address 0x55555555555565 for mLock
      if (mSafeToEnterCb) {
          mCbCount++;
    #ifdef USE_DEBUG
          if (mCbCount > 1) {
              SL_LOGV("Callback protector allowed multiple or nested callback entry: %u", mCbCount);
          } else {
              mCallbackThread = pthread_self();
              mCallbackTid = gettid();
          }
    #endif
      } else {
    #ifdef USE_DEBUG
          SL_LOGV("Callback protector denied callback entry by thread %p tid %d during destroy"
                  " requested by thread %p tid %d",
                  (void *) pthread_self(), gettid(),
                  (void *) mRequesterThread, mRequesterTid);
    #else
          SL_LOGV("Callback protector denied callback entry during destroy");
    #endif
      }
      return mSafeToEnterCb;
    }

    mLock was freed when come into enterCb().

  2. It seems that the mCallbackProtector was freed just after if (protector != 0), that is the opensles recorder was destroy almost at the same time.

    void android_audioRecorder_destroy(CAudioRecorder* ar) {
      SL_LOGV("android_audioRecorder_destroy(%p) entering", ar);
    
      if (ar->mAudioRecord != 0) {
          ar->mAudioRecord->stop();
          ar->mAudioRecord.clear();
     }
      // explicit destructor
      ar->mAudioRecord.~sp();
      ar->mCallbackProtector.~sp();
    }

    3 . I think the reason is here enterCbIfOk(const sp<CallbackProtector> &protector) , the parameter protector is a reference, so it can not influence the free of mCallbackProtector. So i wonder can we change it as wp or just const sp<CallbackProtector> protector?

philburk commented 4 months ago

I agree with your analysis. The code does seem dangerous. I do not like the explicit calls to the destructors in android_audioRecorder_destroy.

But we cannot change Android 10. And OpenSL ES is deprecated. So we cannot fix this bug.

I recommend using AAudio with Oboe on Android 9 and above.

@lhran99 - Is there a reason you are explicitly requesting OpenSL ES?

lhran99 commented 4 months ago

I agree with your analysis. The code does seem dangerous. I do not like the explicit calls to the destructors in android_audioRecorder_destroy.

But we cannot change Android 10. And OpenSL ES is deprecated. So we cannot fix this bug.

I recommend using AAudio with Oboe on Android 9 and above.

@lhran99 - Is there a reason you are explicitly requesting OpenSL ES?

We did want to use aaudio, but we found that some mobile devices have noise problems with aaudio, but works fine with opensles。

philburk commented 4 months ago

We did want to use aaudio, but we found that some mobile devices have noise problems with aaudio

Oh! Could you please file a separate bug for that? OpenSL ES is deprecated so we want to make sure AAudio does not have any problems.

philburk commented 2 months ago

I am reopening this issue because I want to fully understand the bug and the proposed fix.

flamme commented 2 months ago

An internal tracking bug is b/361799177

philburk commented 2 months ago

Interesting similar bug at https://github.com/kcat/openal-soft/issues/10

@lhran99 - thanks again for reporting this.

So i wonder can we change it as wp

Good idea. That may work.

We discussed this and think that a simpler fix would be to force join the callbacks.

      ar->mAudioRecord->stop();
      ar->mAudioRecord->stopAndJoinCallbacks();

It is an easy fix but will require a lot of testing.