Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
lhran99 opened this issue Jun 11, 2024 · 6 comments
Assignees
Labels

Comments

@lhran99
Copy link

lhran99 commented Jun 11, 2024

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)
#1 pc 0000000000020900 /system/lib64/libwilhelm.so (_ZN7android17CallbackProtector11enterCbIfOkERKNS_2spIS0_EE+32)
#2 pc 000000000001e534 /system/lib64/libwilhelm.so (ZL22audioRecorder_callbackiPvS+44)
#3 pc 0000000000051e08 /system/lib64/libaudioclient.so (_ZN7android11AudioRecord18processAudioBufferEv+1184)
#4 pc 0000000000051670 /system/lib64/libaudioclient.so (_ZN7android11AudioRecord17AudioRecordThread10threadLoopEv+264)
#5 pc 00000000000137c0 /system/lib64/libutils.so (_ZN7android6Thread11_threadLoopEPv+312)
#6 pc 00000000000c3814 /system/lib64/libandroid_runtime.so (_ZN7android14AndroidRuntime15javaThreadShellEPv+140)
#7 pc 00000000000e2364 /apex/com.android.runtime/lib64/bionic/libc.so (_ZL15__pthread_startPv+36)
#8 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().

  1. 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?

@lhran99 lhran99 added the bug label Jun 11, 2024
@philburk
Copy link
Collaborator

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
Copy link
Author

lhran99 commented Jun 28, 2024

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
Copy link
Collaborator

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 philburk reopened this Aug 23, 2024
@philburk
Copy link
Collaborator

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

@flamme
Copy link
Collaborator

flamme commented Aug 23, 2024

An internal tracking bug is b/361799177

@philburk
Copy link
Collaborator

Interesting similar bug at kcat/openal-soft#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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants