boinx / DeckLink

ObjC wrapper for the Blackmagic Design DeckLink SDK
MIT License
15 stars 12 forks source link

Possible deadlock on playback #9

Open lov opened 2 years ago

lov commented 2 years ago

Hi guys,

in DeckLinkDevice+Playback.mm file there are 3 methods, that using the internal serial playbackQueue synchronously:

There is a possible problem of these calls: if you are calling any them on the main thread, and the execution is longer than a few milliseconds, this could deadlock the main thread. On the other hand, getting this calls async should not introduce any internal problems, since playbackQueue will maintain order anyway. However, this would need change of the methods, because they have a return value.

By the way, I see this issue in real life, although it is hardly reproducible - but could be happening when a device disconnects and reconnects during playback on my end.

boinxbob commented 2 years ago

Tamas,

thank you for pointing this out. calling stuff synchronously is not always a good idea. So maybe it make sense to come up with another method that does the calls asynchronously and with a completion handler.

However, please, can you take a process sample while the main thread is blocked so we can see the dependency from the calls made in the synchronous block to the main thread? If you think this is a race condition you may add an [NSThread sleepForTimeInterval:1.0] in your code to provoke the deadlock.

best,

Achim

On 05.03.2022, at 16:43, Tamas Nagy @.***> wrote:

Hi guys,

in DeckLinkDevice+Playback.mm file there are 3 methods, that using the internal serial playbackQueue synchronously:

setPlaybackActiveVideoFormatDescription setPlaybackActiveAudioFormatDescription setPlaybackActiveKeyingMode There is a possible problem of these calls: if you are calling any them on the main thread, and the execution is longer than a few milliseconds, this could deadlock the main thread. On the other hand, getting this calls async should not introduce any internal problems, since playbackQueue will maintain order anyway. However, this would need change of the methods, because they have a return value.

By the way, I see this issue in real life, although it is hardly reproducible - but could be happening when a device disconnects and reconnects during playback on my end.

— Reply to this email directly, view it on GitHub https://github.com/boinx/DeckLink/issues/9, or unsubscribe https://github.com/notifications/unsubscribe-auth/AATHCIDCPNU2R3VYPYGU2LDU6N6KHANCNFSM5P72XQJQ. Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub. You are receiving this because you are subscribed to this thread.

lov commented 2 years ago

Hi Achim,

I'm attaching a sample log of the problem from our app - there is a lot other stuff going on, so I've removed everything not Decklink related from the sample.

Since my starter post, I further investigated the problem, and my finding is in a nutshell°, the deadlock is happening because the playbackQueue gets stuck, then the main thread deadlocks because it trying to call sync on that queue.

I already added completed handlers around this in my fork, should I make a PR?

° = The problem originates from a possible driver problem - com.blackmagic-design.BlackmagicIO.DExt can be stuck on disconnecting the hardware during playback, although it is rarely reproducible, but if it happens, then when the device reconnects, it will be stuck on calls like DisplayFrameSync() for any apps on the system until you kill that process. But although the problem is originates from this, it would be good to remove the possibility of getting a deadlock on other threads, especially on the main.

Sample of Decklink.txt