[vlc-devel] [PATCH 7/7] win32/window: remove mutex, fix heap corruption
Steve Lhomme
robux4 at ycbcr.xyz
Tue Feb 4 09:32:54 CET 2020
On 2020-02-03 22:13, RĂ©mi Denis-Courmont wrote:
> A lock is overkill to swap a variable: use an atomic variable.
> Also fix a corner case double free (and racy memory read) at exit,
> by freeing the outstanding title only after the thread exits.
It would be even easier to pass a string pointer to WM_VLC_CHANGE_TEXT
and release it when the message is handled.
While _Atomic is technically C11, it was only added to ICC in 2017 and
there might be compilers with even less support. We currently don't use
it anywhere in the code.
> ---
> modules/video_output/win32/window.c | 21 +++++----------------
> 1 file changed, 5 insertions(+), 16 deletions(-)
>
> diff --git a/modules/video_output/win32/window.c b/modules/video_output/win32/window.c
> index f2d85e8c0b..2291a323af 100644
> --- a/modules/video_output/win32/window.c
> +++ b/modules/video_output/win32/window.c
> @@ -28,6 +28,7 @@
> # include <config.h>
> #endif
>
> +#include <stdatomic.h>
> #include <stdarg.h>
>
> #include <vlc_common.h>
> @@ -50,7 +51,6 @@
> typedef struct vout_window_sys_t
> {
> vlc_thread_t thread;
> - vlc_mutex_t lock;
> vlc_sem_t ready;
>
> HWND hwnd;
> @@ -73,7 +73,7 @@ typedef struct vout_window_sys_t
> LONG i_window_style;
>
> /* Title */
> - wchar_t *pwz_title;
> + wchar_t *_Atomic pwz_title;
> } vout_window_sys_t;
>
>
> @@ -146,11 +146,7 @@ static void SetTitle(vout_window_t *wnd, const char *title)
> if (unlikely(pwz_title == NULL))
> return;
>
> - vlc_mutex_lock( &sys->lock );
> - free( sys->pwz_title );
> - sys->pwz_title = pwz_title;
> - vlc_mutex_unlock( &sys->lock );
> -
> + free(atomic_exchange(&sys->pwz_title, pwz_title));
> PostMessage( sys->hwnd, WM_VLC_CHANGE_TEXT, 0, 0 );
> }
>
> @@ -453,12 +449,7 @@ static long FAR PASCAL WinVoutEventProc( HWND hwnd, UINT message,
> case WM_VLC_CHANGE_TEXT:
> {
> vout_window_sys_t *sys = wnd->sys;
> - wchar_t *pwz_title;
> -
> - vlc_mutex_lock( &sys->lock );
> - pwz_title = sys->pwz_title;
> - sys->pwz_title = NULL;
> - vlc_mutex_unlock( &sys->lock );
> + wchar_t *pwz_title = atomic_exchange(&sys->pwz_title, NULL);
>
> if( pwz_title )
> {
> @@ -476,13 +467,12 @@ static void Close(vout_window_t *wnd)
> {
> vout_window_sys_t *sys = wnd->sys;
>
> - free( sys->pwz_title );
> if (sys->hwnd)
> PostMessage( sys->hwnd, WM_CLOSE, 0, 0 );
>
> vlc_join(sys->thread, NULL);
> vlc_sem_destroy( &sys->ready );
> - vlc_mutex_destroy( &sys->lock );
> + free(atomic_load(&sys->pwz_title));
>
> if (sys->hwnd)
> DestroyWindow( sys->hwnd );
> @@ -729,7 +719,6 @@ static int Open(vout_window_t *wnd)
> msg_Err( wnd, "RegisterClass FAILED (err=%lu)", GetLastError() );
> return VLC_EGENERIC;
> }
> - vlc_mutex_init( &sys->lock );
> vlc_sem_init( &sys->ready, 0 );
>
> wnd->sys = sys;
> --
> 2.25.0
>
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel
>
More information about the vlc-devel
mailing list