[vlc-commits] [Git][videolan/vlc][master] 10 commits: transcode: video: fix lock/unlock imbalance on error
Steve Lhomme (@robUx4)
gitlab at videolan.org
Tue Sep 22 07:44:15 UTC 2026
Steve Lhomme pushed to branch master at VideoLAN / VLC
Commits:
a02d2910 by Alexandre Janniaux at 2026-09-22T07:27:57+00:00
transcode: video: fix lock/unlock imbalance on error
- - - - -
9c3f88ee by Alexandre Janniaux at 2026-09-22T07:27:57+00:00
misc: fsstorage: fix lock/unlock imbalance on error
- - - - -
31758ac5 by Alexandre Janniaux at 2026-09-22T07:27:57+00:00
httpd: fix lock/unlock imbalance on error
- - - - -
95b013e1 by Alexandre Janniaux at 2026-09-22T07:27:57+00:00
d3d11_surface: fix lock/unlock imbalance on error
- - - - -
e192b982 by Alexandre Janniaux at 2026-09-22T07:27:57+00:00
stream_out: bridge: fix lock/unlock imbalance on error
- - - - -
0b0080ae by Alexandre Janniaux at 2026-09-22T07:27:57+00:00
freetype: fontconfig: fix lock/unlock imbalance on error
- - - - -
319dffb5 by Alexandre Janniaux at 2026-09-22T07:27:57+00:00
bluray: fix lock/unlock imbalance in case of error
- - - - -
fb2f726c by Alexandre Janniaux at 2026-09-22T07:27:57+00:00
bluray: rework how lock works for overlay
The helper function for locking the overlay were also locking the
updater lock, and the locking function would change it's behaviour
depending on whether an overlay is available or not, but the unlocking
function would not, which is weird.
In addition, those two helpers functions were used only at two
locations, so it's easier to inline the locking, and will make it easier
to apply semantic patching to check the lock balancing.
The initial reason for looking into this is that it was reported as a
false positive for the lock balancing check "in case of error" as it is
written with the fact in mind that locking functions are not excepted to
"fail" or change behavior.
- - - - -
9d3abcf3 by Alexandre Janniaux at 2026-09-22T07:27:57+00:00
extras: spatch: add semantic patch for vlc_mutex
Add a semantic patch to detect misuage of locking where it is not
unlocked on error. The file could be extended for other lock use cases
later.
It builds on the lock/unlock imbalance[^Exemple.4] from vayavyalabs. I
had seen the transcode lock imbalance issue and was wondering how to
detect such case.
Compared to vayavyalabs's proposition in the blog, it also avoids
applying this semantic to "locked" functions, where the function is
called with an already locked mutex and might unlock/relock in the
middle of the function, which would lead to an additional unlock without
the @unlockedfn position filter (and false positive in mediacodec with
the spatch).
This patch does not yet detect missing lock in the case mentioned above,
nor when goto is used instead of return, since the current patch already
unveil some issues. They can probably be added here later.
Usage:
spatch \
--sp-file extras/spatch/lock.cocci \
--dir modules/ \
--include-headers \
--jobs 8 \
--patch .
[^Exemple.4]: https://vayavyalabs.com/blogs/coccinelle-the-semantic-patch-master/
- - - - -
41643e2a by Alexandre Janniaux at 2026-09-22T07:27:57+00:00
freetype: fontconfig: fix memory issues
Many memory issues were present as soon as failures happened in the
FontConfig_Prepare(). In the non-windows case, config was leaked if
FcConfigBuildFonts failed and the dialog was not cancelled, while in
addition it would be leaking a refs preventing future loading.
This rework the ordering after previous commit fixing the unlocking on
return/
- - - - -
8 changed files:
- + extras/spatch/lock.cocci
- modules/access/bluray.c
- modules/hw/d3d11/d3d11_surface.c
- modules/misc/addons/fsstorage.c
- modules/stream_out/bridge.c
- modules/stream_out/transcode/video.c
- modules/text_renderer/freetype/fonts/fontconfig.c
- src/network/httpd.c
Changes:
=====================================
extras/spatch/lock.cocci
=====================================
@@ -0,0 +1,39 @@
+@ alreadylocked exists @
+expression lock;
+identifier fn;
+position lockedfn;
+@@
+ fn(...) {
+ ...when != vlc_mutex_lock(&lock)
+ vlc_mutex_unlock(&lock);
+ ...
+ vlc_mutex_lock at lockedfn(&lock);
+ ...
+ }
+@ nobrace exists @
+expression lock;
+expression condition;
+position p;
+position unlockedfn != alreadylocked.lockedfn;
+@@
+ vlc_mutex_lock at unlockedfn(&lock);
+ ...when != vlc_mutex_unlock(&lock);
+ if at p (condition)
++{
++ vlc_mutex_unlock(&lock);
+ return ...;
++}
+
+@ braced exists @
+expression lock;
+expression condition;
+position p != nobrace.p;
+position unlockedfn != alreadylocked.lockedfn;
+@@
+ vlc_mutex_lock at unlockedfn(&lock);
+ ...when != vlc_mutex_unlock(&lock);
+ if at p (condition) {
+ ...when != vlc_mutex_unlock(&lock);
++ vlc_mutex_unlock(&lock);
+ return ...;
+ }
=====================================
modules/access/bluray.c
=====================================
@@ -230,6 +230,8 @@ typedef struct bluray_spu_updater_sys_t bluray_spu_updater_sys_t;
typedef struct bluray_overlay_t
{
+ /* this lock is held while vout accesses overlay. => overlay can't
+ * be modified. */
vlc_mutex_t lock;
bool b_on_vout;
OverlayStatus status;
@@ -1560,46 +1562,27 @@ static es_out_t *esOutNew(vlc_object_t *p_obj, es_out_t *p_dst_out, void *priv)
* subpicture_updater_t functions:
*****************************************************************************/
-static bluray_overlay_t *updater_lock_overlay(bluray_spu_updater_sys_t *p_upd_sys)
-{
- /* this lock is held while vout accesses overlay. => overlay can't be closed. */
- vlc_mutex_lock(&p_upd_sys->lock);
-
- bluray_overlay_t *ov = p_upd_sys->p_overlay;
- if (ov) {
- /* this lock is held while vout accesses overlay. => overlay can't be modified. */
- vlc_mutex_lock(&ov->lock);
- return ov;
- }
-
- /* overlay has been closed */
- vlc_mutex_unlock(&p_upd_sys->lock);
- return NULL;
-}
-
-static void updater_unlock_overlay(bluray_spu_updater_sys_t *p_upd_sys)
-{
- assert (p_upd_sys->p_overlay);
-
- vlc_mutex_unlock(&p_upd_sys->p_overlay->lock);
- vlc_mutex_unlock(&p_upd_sys->lock);
-}
-
static void subpictureUpdaterUpdate(subpicture_t *p_subpic,
const struct vlc_spu_updater_configuration *cfg)
{
VLC_UNUSED(cfg);
bluray_spu_updater_sys_t *p_upd_sys = p_subpic->updater.sys;
- bluray_overlay_t *p_overlay = updater_lock_overlay(p_upd_sys);
- if (!p_overlay) {
+ vlc_mutex_lock(&p_upd_sys->lock);
+ bluray_overlay_t *p_overlay = p_upd_sys->p_overlay;
+
+ if (p_overlay == NULL) {
+ vlc_mutex_unlock(&p_upd_sys->lock);
return;
}
+ vlc_mutex_lock(&p_overlay->lock);
+
if (p_overlay->status != Outdated)
{
- updater_unlock_overlay(p_upd_sys);
+ vlc_mutex_unlock(&p_overlay->lock);
+ vlc_mutex_unlock(&p_upd_sys->lock);
return;
}
@@ -1627,21 +1610,28 @@ static void subpictureUpdaterUpdate(subpicture_t *p_subpic,
}
p_overlay->status = Displayed;
- updater_unlock_overlay(p_upd_sys);
+ vlc_mutex_unlock(&p_overlay->lock);
+ vlc_mutex_unlock(&p_upd_sys->lock);
}
static void subpictureUpdaterDestroy(subpicture_t *p_subpic)
{
bluray_spu_updater_sys_t *p_upd_sys = p_subpic->updater.sys;
- bluray_overlay_t *p_overlay = updater_lock_overlay(p_upd_sys);
- if (p_overlay) {
- /* vout is closed (seek, new clip, ?). Overlay must be redrawn. */
- p_overlay->status = ToDisplay;
- p_overlay->b_on_vout = false;
- updater_unlock_overlay(p_upd_sys);
- }
+ vlc_mutex_lock(&p_upd_sys->lock);
+ bluray_overlay_t *p_overlay = p_upd_sys->p_overlay;
+ if (p_overlay == NULL)
+ goto end;
+
+ vlc_mutex_lock(&p_overlay->lock);
+ /* vout is closed (seek, new clip, ?). Overlay must be redrawn. */
+ p_overlay->status = ToDisplay;
+ p_overlay->b_on_vout = false;
+ vlc_mutex_unlock(&p_overlay->lock);
+
+end:
+ vlc_mutex_unlock(&p_upd_sys->lock);
unref_subpicture_updater(p_upd_sys);
}
@@ -1926,8 +1916,10 @@ static void blurayOverlayProc(void *ptr, const BD_OVERLAY *const overlay)
return;
}
- if(overlay->plane >= MAX_OVERLAY)
+ if(overlay->plane >= MAX_OVERLAY) {
+ vlc_mutex_unlock(&p_sys->bdj.lock);
return;
+ }
switch (overlay->cmd) {
case BD_OVERLAY_INIT:
=====================================
modules/hw/d3d11/d3d11_surface.c
=====================================
@@ -252,8 +252,10 @@ static void D3D11_YUY2(filter_t *p_filter, picture_t *src, picture_t *dst)
if (sys->d3d_proc.procEnumerator)
{
HRESULT hr;
- if (FAILED( D3D11_Assert_ProcessorInput(p_filter, &sys->d3d_proc, p_sys) ))
+ if (FAILED( D3D11_Assert_ProcessorInput(p_filter, &sys->d3d_proc, p_sys) )) {
+ vlc_mutex_unlock(&sys->staging_lock);
return;
+ }
D3D11_VIDEO_PROCESSOR_STREAM stream = {
.Enable = TRUE,
@@ -366,8 +368,10 @@ static void D3D11_NV12(filter_t *p_filter, picture_t *src, picture_t *dst)
if (sys->d3d_proc.procEnumerator)
{
HRESULT hr;
- if (FAILED( D3D11_Assert_ProcessorInput(p_filter, &sys->d3d_proc, p_sys) ))
+ if (FAILED( D3D11_Assert_ProcessorInput(p_filter, &sys->d3d_proc, p_sys) )) {
+ vlc_mutex_unlock(&sys->staging_lock);
return;
+ }
D3D11_VIDEO_PROCESSOR_STREAM stream = {
.Enable = TRUE,
=====================================
modules/misc/addons/fsstorage.c
=====================================
@@ -856,7 +856,10 @@ static int Remove( addons_storage_t *p_storage, addon_entry_t *p_entry )
char *psz_translated_filename = strdup( p_file->psz_filename );
if ( !psz_translated_filename )
+ {
+ vlc_mutex_unlock( &p_entry->lock );
return VLC_ENOMEM;
+ }
char *tmp = psz_translated_filename;
while (*tmp++) if ( *tmp == '/' ) *tmp = DIR_SEP_CHAR;
@@ -866,6 +869,7 @@ static int Remove( addons_storage_t *p_storage, addon_entry_t *p_entry )
{
free( psz_dir );
free( psz_translated_filename );
+ vlc_mutex_unlock( &p_entry->lock );
return VLC_EGENERIC;
}
free( psz_dir );
=====================================
modules/stream_out/bridge.c
=====================================
@@ -216,7 +216,10 @@ static void *AddOut( sout_stream_t *p_stream, const es_format_t *p_fmt, const ch
bridged_es_id = NULL;
if ( unlikely(bridged_es_id == NULL) )
+ {
+ vlc_mutex_unlock( &lock );
return NULL;
+ }
if ( i == p_bridge->i_es_num )
{
=====================================
modules/stream_out/transcode/video.c
=====================================
@@ -140,12 +140,16 @@ static int video_update_format_decoder( decoder_t *p_dec, vlc_video_context *vct
struct encoder_owner *p_enc_owner =
(struct encoder_owner *)sout_EncoderCreate( VLC_OBJECT(p_owner->p_stream), sizeof(struct encoder_owner) );
if ( unlikely(p_enc_owner == NULL))
+ {
+ vlc_mutex_unlock(&id->fifo.lock);
return VLC_EGENERIC;
+ }
id->encoder = transcode_encoder_new( &p_enc_owner->enc, &p_dec->fmt_out );
if( !id->encoder )
{
vlc_object_delete( &p_enc_owner->enc );
+ vlc_mutex_unlock(&id->fifo.lock);
return VLC_EGENERIC;
}
=====================================
modules/text_renderer/freetype/fonts/fontconfig.c
=====================================
@@ -51,6 +51,7 @@ static vlc_mutex_t lock = VLC_STATIC_MUTEX;
int FontConfig_Prepare( vlc_font_select_t *fs )
{
+ int ret = VLC_SUCCESS;
vlc_tick_t ts;
vlc_mutex_lock( &lock );
@@ -66,12 +67,15 @@ int FontConfig_Prepare( vlc_font_select_t *fs )
#ifndef _WIN32
config = FcInitLoadConfigAndFonts();
if( unlikely(config == NULL) )
- refs = 0;
+ ret = VLC_ENOMEM;
#else
- unsigned int i_dialog_id = 0;
- dialog_progress_bar_t *p_dialog = NULL;
config = FcInitLoadConfig();
+ if( unlikely(config == NULL) )
+ {
+ ret = VLC_ENOMEM;
+ goto end;
+ }
int i_ret =
vlc_dialog_display_progress( fs->p_obj, true, 0.0, NULL,
@@ -79,20 +83,26 @@ int FontConfig_Prepare( vlc_font_select_t *fs )
_("Please wait while your font cache is rebuilt.\n"
"This should take less than a few minutes.") );
- i_dialog_id = i_ret > 0 ? i_ret : 0;
+ unsigned int i_dialog_id = i_ret > 0 ? i_ret : 0;
if( FcConfigBuildFonts( config ) == FcFalse )
- return VLC_ENOMEM;
+ {
+ FcConfigDestroy(config);
+ ret = VLC_ENOMEM;
+ }
if( i_dialog_id != 0 )
vlc_dialog_cancel( fs->p_obj, i_dialog_id );
#endif
+ if (ret != VLC_SUCCESS)
+ refs--;
+
vlc_mutex_unlock( &lock );
msg_Dbg( fs->p_obj, "Took %" PRId64 " microseconds", vlc_tick_now() - ts );
- return (config != NULL) ? VLC_SUCCESS : VLC_EGENERIC;
+ return ret;
}
void FontConfig_Unprepare( vlc_font_select_t *fs )
=====================================
src/network/httpd.c
=====================================
@@ -1125,6 +1125,7 @@ error:
free(url->psz_url);
free(url);
+ vlc_mutex_unlock(&host->lock);
return NULL;
}
View it on GitLab: https://code.videolan.org/videolan/vlc/-/compare/d5c978b6800292ddc177eadcc9c06d1b5352a92e...41643e2ae0c147df7e61f8a6ec8e9d2426f278dc
--
View it on GitLab: https://code.videolan.org/videolan/vlc/-/compare/d5c978b6800292ddc177eadcc9c06d1b5352a92e...41643e2ae0c147df7e61f8a6ec8e9d2426f278dc
You're receiving this email because of your account on code.videolan.org. Manage all notifications: https://code.videolan.org/-/profile/notifications | Help: https://code.videolan.org/help
More information about the vlc-commits
mailing list