Skip to content

Fix deadlock in MediaPlayer.#900

Merged
overtake merged 1 commit intoovertake:masterfrom
walkline:audio-renderer-deadlock
Dec 27, 2022
Merged

Fix deadlock in MediaPlayer.#900
overtake merged 1 commit intoovertake:masterfrom
walkline:audio-renderer-deadlock

Conversation

@walkline
Copy link
Contributor

@walkline walkline commented Dec 15, 2022

There is an issue with MediaPlayer.

We have at least 3 bug reports that covers that:
https://bugs.telegram.org/c/20486
https://bugs.telegram.org/c/21311
#892

Personally I like description of this one.

But in general when the video ends or when you rewind it, there is a chance that all videos stops to play (until you restart the app) or the main thread deadlocks (app just hangs) and you need to restart the application.

It's easy to reproduce on Macbook with 2,4 GHz 8-Core Intel Core i9. But I couldn't reproduce it on Macbook Air with 2 slow cores.

The reason of that behaviour is deadlock that occurs when using some CoreMedia components (from my observation CM clock or time related functions, like CMAudioDeviceClockCreate).

I made an assumption that this can occur because of the race condition when we create MediaPlayerAudioRenderer. So we creating it from playerQueue queue and there is a chance that in audioPlayerRendererQueue there are still tasks related to cleaning up of the previous object. So we need to wait until all cleaning happens and then create required objects.

With suggested changes I couldn't reproduce the issue.

Let me know if you need more details, I can provide threads stacktrace with deadlock, or video to reproduce this bug.

@CLAassistant
Copy link

CLAassistant commented Dec 15, 2022

CLA assistant check
All committers have signed the CLA.

@overtake
Copy link
Owner

Interesting. I will look closer and update next beta with fix. let's see if its help

@walkline
Copy link
Contributor Author

@overtake can't reproduce the issue in the new 9.3 beta. Some changes from your side?

@overtake
Copy link
Owner

@overtake can't reproduce the issue in the new 9.3 beta. Some changes from your side?

Yes. I've implemented your fix. So, if there is no more issue i merge your pull request. Thank you.

@overtake overtake merged commit 67e4cf8 into overtake:master Dec 27, 2022
@overtake
Copy link
Owner

i will figure out later how to fix this issue properly =)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

4 participants