LibVLC Deadlock on libvlc_media_player stop (win32) [Fixed]

[Rumpel]

solud can you show me a littel bit of code form you as example??

[solud]

Code: Select all

private void StopVideo() { VlcMediaPlayer p1 = player1MediaPlayer; VlcMediaPlayer p2 = player2MediaPlayer; if (p1 != null) p1.Stop(); if (p2 != null) p2.Stop(); } private void VideoForm_FormClosing(object sender, FormClosingEventArgs e) { Thread t = new Thread(new ThreadStart(StopVideo)); t.Start(); }

[Rumpel]

Thank you

[rvs75]

Seems to work for me too. I add a "t.join()" after "t.start()" because i have a libvlc_media_player_set_media, libvlc_media_player_set_hwnd and libvlc_media_player_play after the libvlc_media_player_stop.

Thanx

[Rumpel]

I have still problems but i think it is the right way.

EDIT:
I have a question. So i get no deadlocks but in some cases if i will stop a video the libvlc_media_player_stop() function needs exactly 30sec to close but only in a few cases not all. Why?

EDIT 2:
Sorry now i read in the forum that RTSP is very slow sometimes but in thread is no solution. Any tipps?

[Rémi Denis-Courmont]

For RTSP, there is no out-of-the-box solution. Either live555 needs to be fixed, or the VLC RTSP input must be rewritten to use a better library.

[Sébastien Escudier]

this should be fixed in 1.2

[bratao]

Sébastien, whats will be fixed, the deadlock problem or the RTSP one ?

[Rémi Denis-Courmont]

RTSP. I don't see how VLC would fix a limitation of Win32...

[bratao]

Rémi, if fact you can fix it in libvlc. Just replacing the WaitForSingleObjectEx for a MsgWait equivalent:

Code: Select all

ret = MsgWaitForMultipleObjects (1, &handle, INFINITE, NULL, NULL); if (ret == WAIT_OBJECT_0 + 1) { /* PROCESS THE MESSAGE */ BOOL bRet; while ((bRet = PeekMessage (&msg, NULL, 0, 0, PM_REMOVE)) != 0) { TranslateMessage (&msg); DispatchMessage (&msg); } LOOP again until we receive WAIT_OBJECT_0 }

[Rémi Denis-Courmont]

That might be a good idea. That would be then MsgWaitForMultipleObjectsEx because we need an alertable function.

However, I'm a bit suspicious that this will raise surprise reentrancy problems in VLC and VLC-based applications :-/

[ispy]

If this could be fixed it would help my project a lot - i get about 20 error reports of VLC deadlocking on stop a day. I've tried every workaround I can find/ think of.

[Rémi Denis-Courmont]

I am not an expert in Win32 API and I might have missed something but...

• It is not clear to me that the Win32 documentation would require the use of MsgWaitForMultipleObjects(Ex) instead of WaitForMultipleObjects(Ex), WaitForSingleObject(Ex) and Sleep(Ex) in threads that have created windows. In particular, LibVLC should not need any window messages to wait for another thread to complete.
• The proposed change may reentrancy problems in the window events procedures that did not get called from LibVLC and now would get called.
• It might interfere with the main loop of the applications, if they did anything other than just TranslateMessage/DispatchMessage.
• Nobody has reviewied the proposed patch on vlc-devel.

So at this point, I am going to assume that this is a bug in some applications and not in LibVLC. The ball is yours.

[bratao]

I think that this path is very important.

It is not clear to me that the Win32 documentation would require the use of MsgWaitForMultipleObjects(Ex) instead of WaitForMultipleObjects(Ex), WaitForSingleObject(Ex) and Sleep(Ex) in threads that have created windows. In particular, LibVLC should not need any window messages to wait for another thread to complete.

The documentation cleary state that if a thread process messages, it NEED use MsgWaitForMultipleObjects
From http://msdn.microsoft.com/en-us/library ... 85%29.aspx

If a thread creates any windows, it must process messages. Message broadcasts are sent to all windows in the system. A thread that uses a wait function with no time-out interval may cause the system to become deadlocked. Two examples of code that indirectly creates windows are DDE and the CoInitialize function. Therefore, if you have a thread that creates windows, use MsgWaitForMultipleObjects or MsgWaitForMultipleObjectsEx, rather than WaitForMultipleObjects.

The proposed change may reentrancy problems in the window events procedures that did not get called from LibVLC and now would get called.

I don´t see how this could generate any reentrancy problems. Could explain it ?

It might interfere with the main loop of the applications, if they did anything other than just TranslateMessage/DispatchMessage.

No you got it wrong. The main loop of applications could do anything, if it not create window, MsgWait.. will behave exactly as WaitForMultipleObjects.

Nobody has reviewied the proposed patch on vlc-devel.

The patch is here : http://mailman.videolan.org/pipermail/v ... 81125.html
It is wrong. With this code, i understand why you should see that it may have reentrancy problems, and if fact, they will.
The MsgWaitForMultipleObjects should wait normaly as WaitForMultipleObjects but if it recieve a message, it should process it, and then go back to wait state.
This way, if a thread do not create windows it works exactly as WaitForMultipleObjects. If it create a windows, it NEED MsgWaitForMultipleObjects.
The patch should be in like this:

Code: Select all

--- src/win32/thread.c | 14 +++++++++++++- 1 files changed, 13 insertions(+), 1 deletions(-) diff --git a/src/win32/thread.c b/src/win32/thread.c index 00c2781..f4521b1 100644 --- a/src/win32/thread.c +++ b/src/win32/thread.c @@ -125,7 +125,19 @@ BOOL WINAPI DllMain (HINSTANCE hinstDll, DWORD fdwReason, LPVOID lpvReserved) static DWORD vlc_WaitForMultipleObjects (DWORD count, const HANDLE *handles, DWORD delay) { - DWORD ret = WaitForMultipleObjectsEx (count, handle, FALSE, delay, TRUE); + DWORD ret; + for (;;) + { + ret = MsgWaitForMultipleObjectsEx (count, handle, delay, + QS_ALLINPUT, MWMO_ALTERTABLE); + if (ret == WAIT_OBJECT_0 + count)/* We have a message to process, dispatch it, then come back to processing */ + { + MSG msg; + + while (PeekMessage (&msg, NULL, 0, 0, PM_REMOVE)) + { + TranslateMessage (&msg); + DispatchMessage (&msg); + } + } + else + break; + } /* We do not abandon objects... this would be a bug */ assert (ret < WAIT_ABANDONED_0 || WAIT_ABONDONED_0 + count - 1 < ret); --

I did´t check the VLC Code, but it objects use to be Abandoned , the code should subtract delay on each loop. But if it really need to wait for the object the code is correctly.
A remark is that on the assert (at last on the maillist patch) have a typo on WAIT_ABONDONED_0.

[Rémi Denis-Courmont]

The documentation cleary state that if a thread process messages, it NEED use MsgWaitForMultipleObjects

The point is, there is no reason why the the thread would have process a message in the first place. Waiting for another VLC thread to exit does not require processing messages to make any progress. Thus the thread should be perfectly fine postponing the message processing until LibVLC unwinds and the calling application main loop is reached.

I don´t see how this could generate any reentrancy problems. Could explain it ?

It's obvious! You'd be calling the window procedure from a site, within LibVLC, where it was not called earlier.

It might interfere with the main loop of the applications, if they did anything other than just TranslateMessage/DispatchMessage.

No you got it wrong. The main loop of applications could do anything, if it not create window, MsgWait.. will behave exactly as WaitForMultipleObjects.

If it does not create a window, then it won't make a difference indeed. But this is a stupid irrelevant point.

If the assumption were that the thread does not create any window, we wouldn't need to have this discussion in the first place...