-
Notifications
You must be signed in to change notification settings - Fork 23
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
Update ndk
and ndk-glue
to 0.7
#47
Conversation
As evidenced by the commits on the branch of this PR, I'm having some difficulties making the project build with NDK 25. |
Okay, I'm now officially lost in the forest of runner image-, Android API-, and NDK-versions... :| |
@torokati44 Could you fix CI? |
Of course |
Yes, that's exactly what I've been trying so hard to do... 😐 I'll try to give it another go sometime, downgrading to NDK 24. |
This mainly fix issue katyo#41, that causes crashes when a `AudioStream` was drop. That happen because the `AudioStream` was not closed on Drop, but was deleted, causing a use-after-free by the not closed `onDataCallback` thread. Also, the method `AudioStreamBuilder::open_stream` was using the deprecated method `openStream(AudioStream*)`, that do not allow deleting the `AudioStream` safely. Replaced it by `openStream(shared_ptr<AudioStream>)`. The deprecated function allowed a use-after-free by the `onErrorCallback` thread. Also, as noted by issue katyo#45, the bindings for `AudioStream::close()` was wrongly bound to the concrete implementation of the base class, instead of calling the virtual method. Also note that currently there is no safe way to delete the `onErrorCallback` of a `AudioStream` in oboe (see google/oboe#1610), so instead the current implementation leaks the callback on drop. Also, remove some unsound `Drop` implementations and replace them by explicit unsafe delete methods.
I don't plan to work on this anymore, sorry... :| |
@torokati44 Don't worry about it. I planned to update this library soon. |
@katyo is this something that can be picked up again? Looks like this PR contains much more than simply bumping the |
Re: #46
I haven't actually tested whether this breaks anything, but it shouldn't:registered::tm:.