Closed la-matthew-yang closed 2 years ago
setOnCompletionListener
.So are you using threads other than main in the app ? If not, are you using setOnCompletionListener
method ?
I think I have finally figured out what is causing this issue. :)
Essentially, it is because completion listener is notified after OboeMusic.dispose()
is already called. When this happens, calling jvm_class(context->GetObjectClass(new_callback))
causes JNI ERROR: use of deleted global reference
.
I am not sure how this is possible. Because OboeMusic.stop()
is always called before OboeMusic.dispose()
. Maybe OboeMusic.stop()
has no effect when the music is very close to completion?
I tried to solve this issue by calling OboeMusic.setOnCompletionListener(null)
before OboeMusic.dispose()
. But it seems that OboeMusic.setOnCompletionListener(null)
is not supported.
Oh. It might actually be possible because of OboeMusic.setOnCompletionListener(null)
since it doesn't seems to reset native music callback. Shouldn't be possible after OboeMusic.dispose()
, because it just deletes the native music object (which actually calls the callback), though I'm not sure if this is some thread shenanigans, in that case everything might be possible.
I sadly can't test any of this, because my pc died (and my current can't run the heavy android toolchain). :disappointed:
How often does it crashes ? Are crashes regular or you can't reliably replicate them ? Does it crashes if no OboeMusic.setOnCompletionListener(null)
is called ?
I think you are right about the thread shenanigans. Based on the logs, it seems that AudioTrack::AudioTrackThread::threadLoop()
triggers completion listener right after another thread calls OboeMusic.dispose()
.
Do you think adding a listener check in on_complete could fix the issue?
OBOEMUSIC_METHOD(void, setOnCompletionListener) (JNIEnv* env, jobject self, jobject callback) {
auto context = jni_context::acquire_thread();
if (auto old_callback = get_var_as<_jobject>(env, self, "listener")) {
context->DeleteGlobalRef(old_callback);
}
if (auto instance = get_var_as<std::shared_ptr<music>>(env, self, "music")) {
if (callback) {
auto new_callback = context->NewGlobalRef(callback);
auto self_weak = context->NewWeakGlobalRef(self);
set_var_as(env, self, "listener", new_callback);
(*instance)->on_complete([self_weak, new_callback] {
if (auto new_callback = get_var_as<_jobject>(env, self, "listener")) {
auto context = jni_context::acquire_thread();
auto callback_class = jvm_class(context->GetObjectClass(new_callback));
callback_class.execute_method<void(Music)>(new_callback, "onCompletion", Music{self_weak});
}
});
}
}
}
Off the top of my head, I think something like that might work:
OBOEMUSIC_METHOD(void, setOnCompletionListener) (JNIEnv* env, jobject self, jobject callback) {
auto context = jni_context::acquire_thread();
if (auto old_callback = get_var_as<_jobject>(env, self, "listener")) {
context->DeleteGlobalRef(old_callback);
old_callback = nullptr;
set_var_as<_jobject>(env, self, "listener", nullptr);
}
if (auto instance = get_var_as<std::shared_ptr<music>>(env, self, "music")) {
if (callback) {
auto new_callback = context->NewGlobalRef(callback);
auto self_weak = context->NewWeakGlobalRef(self);
set_var_as(env, self, "listener", new_callback);
(*instance)->on_complete([self_weak, new_callback] {
if (auto new_callback = get_var_as<_jobject>(env, self, "listener")) {
auto context = jni_context::acquire_thread();
auto callback_class = jvm_class(context->GetObjectClass(new_callback));
callback_class.execute_method<void(Music)>(new_callback, "onCompletion", Music{self_weak});
}
});
} else {
(*instance)->on_complete(nullptr);
}
}
}
and
OBOEMUSIC_METHOD(void, dispose) (JNIEnv* env, jobject self) {
if (auto instance = get_var_as<std::shared_ptr<music>>(env, self, "music")) {
(*instance)->on_complete(nullptr);
}
if (auto callback = get_var_as<_jobject>(env, self, "listener")) {
auto context = jni_context::acquire_thread();
context->DeleteGlobalRef(callback);
callback = nullptr;
set_var_as<_jobject>(env, self, "listener", nullptr);
}
delete_var<std::shared_ptr<music>>(env, self, "music");
}
And if that doesn't work, then some spinlocks are required sadly
I think this looks good. I really want to give this a try. But I don't have much experience with NDT or ffmpeg, so I am not sure how long it will take for me to have everything setup. Does the build process work for Mac OS?
Does the build process work for Mac OS?
Yeah, I assume so, yes. You actually don't need anything other than Android NDK and regular android setup. ffmpeg library is pre-built, and normally shouldn't be rebuilt
Thanks! I will give it a try today.
I have the project setup. I am trying to debug the problem. Do you know how to generate the debug symbols so that I can use ndk-stack?
I got ndk-stack working. :) You code is well organized.
After some investigation, I think I have found the root cause of the issue.
It is actually not caused by native completion listener. If I remove the usage of the native completion listener completely, the issue still occurs. Although based on my limited testing, usage of native completion listener increases the chance of the issue occurring.
I think the issue is caused by concurrent modification of audio resources. If music::dispose and music::stop are never called, the issue doesn't occur at all.
The issue probably occurs when a music is being used in mixer::render by the system audio thread, while it is disposed or stopped by a game thread at exactly the time.
The resulting native error has many different stacktraces. Most of them have the message "fault addr". Some have "ConcurrentGC" in their stack trace.
Sometimes, the issue doesn't cause a crash. Instead, it causes Oboe system to stop working. Afterwards, Gdx.audio.newMusic() hands and causes app to get stuck.
I tried putting thread lock in different places, but couldn't get it to work.
The options that I can think of for fixing this issue are:
The second option is great. Don't know why I didn't use shared oboe streams in the first place :shrug:
That may still require a bunch of atomic_flag
tests, though.
I should get my computer fixed soon, I'll look into it. Although, refactoring will likely take some time.
Big thanks for suggestions and tinkering with the code :+1:
@barsoosayque Do you have a preferred way of communication like WhatsApp, Skype, Email, etc? I would like to talk to you about some potential collaboration opportunities, if you don't mind.
Great! I've sent you an email.
I have refactored callback logic in a0ad49bf50d0567938c74831e0bb1ef624141234. And per my tests in 612edb27e8d73f78c50d118df7bea48653134ad6, I can't get any concurent crashes. I want to believe that the issue is somehow fixed, but pretty much open to suggestions on how to test it further.
After some thoughts, I think it's preferable to leave oboe stream as exclusive, since it offers the lowest latency. https://github.com/google/oboe/blob/83f79e93e32dee9e2b5d3ced07dc94c82d4bc182/include/oboe/Definitions.h#L193-L213
Well, I would like to believe that this issue is fixed, so I will close it. If the crash still occurs, please reopen.
I have been struggling with this issue for a while.
I have two questions:
What could potentially cause "use of deleted global reference" in libgdx-oboe?
Is there any way to show function name and line number for libgdx-oboe in backtrace? For all the other so files, there are function name and lines numbers. But for libgdx-oboe (from #12 to #16), there are no such info. Is there a config that I can set to show such info?
Thanks a lot.