[vlc-devel] [PATCH 4/7] win32/window: remove useless done boolean

[Steve Lhomme]

On 2020-02-03 22:13, Rémi Denis-Courmont wrote:
> It was written (and signaled) without proper locking, and it duplicated
> the thread-join anyway since it was the very last observable action.
> ---
>   modules/video_output/win32/window.c | 17 +++--------------
>   1 file changed, 3 insertions(+), 14 deletions(-)
> 
> diff --git a/modules/video_output/win32/window.c b/modules/video_output/win32/window.c
> index 7a91c5dc05..3dc9cf32d2 100644
> --- a/modules/video_output/win32/window.c
> +++ b/modules/video_output/win32/window.c
> @@ -52,7 +52,6 @@ typedef struct vout_window_sys_t
>       vlc_mutex_t  lock;
>       vlc_cond_t   wait;
>       bool         b_ready;
> -    bool         b_done;
>   
>       HWND hwnd;
>   
> @@ -484,22 +483,15 @@ static void Close(vout_window_t *wnd)
>   
>       free( sys->psz_title );
>       if (sys->hwnd)
> -    {
>           PostMessage( sys->hwnd, WM_CLOSE, 0, 0 );
> -        /* wait until the thread is done */
> -        vlc_mutex_lock( &sys->lock );
> -        while( !sys->b_done )
> -        {
> -            vlc_cond_wait( &sys->wait, &sys->lock );
> -        }
> -        vlc_mutex_unlock( &sys->lock );
>   
> -        DestroyWindow( sys->hwnd );
> -    }
>       vlc_join(sys->thread, NULL);
>       vlc_mutex_destroy( &sys->lock );
>       vlc_cond_destroy( &sys->wait );
>   
> +    if (sys->hwnd)
> +        DestroyWindow( sys->hwnd );

You destroy the window after the event loop thread has died. It may have 
so odd side effects. Like keyboard focus issues:
https://docs.microsoft.com/en-us/windows/win32/api/winuser/nf-winuser-destroywindow

The WM_DESTROY we don't handle anymore doesn't seem to be an issue, 
since we're just signaling the event loop thread is dying. (and we 
should handle the WM_QUIT in the thread, which we currently don't)

I haven't seen any issue with this patch applied, but I can't guarantee 
it is clean.

> +
>       HINSTANCE hInstance = GetModuleHandle(NULL);
>       UnregisterClass( sys->class_main, hInstance );
>       DestroyCursor( sys->cursor_empty );
> @@ -691,8 +683,6 @@ static void *EventThread( void *p_this )
>           TranslateMessage(&msg);
>           DispatchMessage(&msg);
>       }
> -    sys->b_done = true;
> -    vlc_cond_signal( &sys->wait );
>       vlc_restorecancel(canc);
>       return NULL;
>   }
> @@ -750,7 +740,6 @@ static int Open(vout_window_t *wnd)
>       vlc_mutex_init( &sys->lock );
>       vlc_cond_init( &sys->wait );
>       sys->b_ready = false;
> -    sys->b_done = false;
>   
>       wnd->sys = sys;
>       if( vlc_clone( &sys->thread, EventThread, wnd, VLC_THREAD_PRIORITY_LOW ) )
> -- 
> 2.25.0
> 
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel
>