Logic race condition in player destructor when switching media/stopping on nearly finished input
I reach assertion
Assertion failed: (!vlc_list_HasInput(&player->destructor.inputs, input)), function vlc_player_destructor_AddInput, file player.c, line 165.Backtrace:
* thread #27, name = 'vlc-input', stop reason = signal SIGABRT
frame #0: 0x0000000180c6d6f4 libsystem_kernel.dylib`__pthread_kill + 8
frame #1: 0x0000000180cfd778 libsystem_pthread.dylib`pthread_kill + 208
frame #2: 0x0000000180bcf0c8 libsystem_c.dylib`abort + 120
frame #3: 0x0000000180bce5e8 libsystem_c.dylib`__assert_rtn + 296
* frame #4: 0x00000001015b2e60 vlccore`vlc_player_destructor_AddInput(player=0x000000010221a1b0, input=0x000000010203f3f0) at player.c:163:9
frame #5: 0x00000001015b2d70 vlccore`vlc_player_destructor_AddJoinableInput(player=0x000000010221a1b0, input=0x000000010203f3f0) at player.c:193:5
frame #6: 0x00000001015babd8 vlccore`input_thread_Events(input_thread=0x0000000102819800, event=0x0000000170562f50, user_data=0x000000010203f3f0) at input.c:864:13
frame #7: 0x00000001015a85dc vlccore`input_SendEvent(p_input=0x0000000102819800, event=0x0000000170562f50) at event.h:35:9
frame #8: 0x00000001015a9608 vlccore`input_SendEventDead(p_input=0x0000000102819800) at event.h:43:5
frame #9: 0x00000001015a7278 vlccore`Run(data=0x0000000102819800) at input.c:486:5
frame #10: 0x0000000180cfc914 libsystem_pthread.dylib`_pthread_start + 116If we check the interleaving, from input/input.c:
static void *Run( void *data )
{
input_thread_private_t *priv = data;
input_thread_t *p_input = &priv->input;
vlc_thread_set_name("vlc-input");
vlc_interrupt_set(&priv->interrupt);
if( !Init( p_input ) )
{
if( priv->master->b_can_pace_control && priv->b_out_pace_control )
{
/* We don't want a high input priority here or we'll
* end-up sucking up all the CPU time */
vlc_set_priority( priv->thread, VLC_THREAD_PRIORITY_LOW );
}
MainLoop( p_input, true ); /* FIXME it can be wrong (like with VLM) */
/* Clean up */
End( p_input );
}
/* ##################### */
input_SendEventDead( p_input );If we call stop right before the execution of lines below #####, we execute vlc_player_AddInput with:
if (input->started)
{
input->started = false;
/* Add this input to the stop list: it will be stopped by the
* destructor thread */
assert(!vlc_list_HasInput(&player->destructor.stopping_inputs, input));
assert(!vlc_list_HasInput(&player->destructor.joinable_inputs, input));
vlc_list_append(&input->node, &player->destructor.inputs);and input_SendEventDead from the input thread will then call vlc_player_AddInput which will execute
else
{
/* Add this input to the joinable list: it will be deleted by the
* destructor thread */
assert(!vlc_list_HasInput(&player->destructor.inputs, input));
assert(!vlc_list_HasInput(&player->destructor.joinable_inputs, input));
vlc_list_append(&input->node, &player->destructor.joinable_inputs);
}except that now, assert(!vlc_list_HasInput(&player->destructor.inputs, input)); is false.
The vlc_player_AddInput from stop or set_media is based on the fact we’ll reach the STOPPED | ENDED state first, which cannot happen if the mainloop is finished.
We didn’t reproduced the issue on desktop and vanilla version because it requires stopping right when the media will be ready to already be stopped, which never happens in practice (either stop is triggered from the user and it stops the media, or the media reaches the end). In my libVLC application, it almost always happens since we virtually “stop” the input media before stopping the media player (high correlation between EOF and stop()). At first I was waiting for the first frame to be displayed, which was masking the problem since I had time to queue data to the input, which will prevent it to stop right away. But when disabling that, the problem is "easy" (for some definition of easy, meaning on device but not simulator typically) to reproduce.
I added logs to assert the issue
diff --git a/src/player/input.c b/src/player/input.c
index 126fde0084..8cc30d7069 100644
--- a/src/player/input.c
+++ b/src/player/input.c
@@ -858,6 +858,8 @@ input_thread_Events(input_thread_t *input_thread,
input_GetItem(input->thread), event->subitems);
break;
case INPUT_EVENT_DEAD:
+ fprintf(stderr, "--- INPUT THREAD: EVENT DEAD\n");
+
if (input->started) /* Can happen with early input_thread fails */
vlc_player_input_HandleState(input, VLC_PLAYER_STATE_STOPPING,
VLC_TICK_INVALID);
diff --git a/src/player/player.c b/src/player/player.c
index 1acaece7df..e5e6059167 100644
--- a/src/player/player.c
+++ b/src/player/player.c
@@ -155,9 +155,11 @@ vlc_player_destructor_AddInput(vlc_player_t *player,
assert(!vlc_list_HasInput(&player->destructor.stopping_inputs, input));
assert(!vlc_list_HasInput(&player->destructor.joinable_inputs, input));
vlc_list_append(&input->node, &player->destructor.inputs);
+ fprintf(stderr, "--- ADDING INPUT TO DESTRUCTOR in INPUTS\n");
}
else
{
+ fprintf(stderr, "--- ADDING INPUT TO DESTRUCTOR in JOINABLES\n");
/* Add this input to the joinable list: it will be deleted by the
* destructor thread */
assert(!vlc_list_HasInput(&player->destructor.inputs, input));
@@ -174,9 +176,13 @@ vlc_player_destructor_AddStoppingInput(vlc_player_t *player,
{
/* Add this input to the stopping list */
if (vlc_list_HasInput(&player->destructor.inputs, input))
+ {
+ fprintf(stderr, "--- REMOVE INPUT FROM DESTRUCTOR in INPUT\n");
vlc_list_remove(&input->node);
+ }
if (!vlc_list_HasInput(&player->destructor.stopping_inputs, input))
{
+ fprintf(stderr, "--- ADD INPUT TO DESTRUCTOR in STOPPING\n");
vlc_list_append(&input->node, &player->destructor.stopping_inputs);
vlc_cond_signal(&input->player->destructor.wait);
}
@@ -223,11 +229,14 @@ vlc_player_destructor_Thread(void *data)
struct vlc_player_input *input;
vlc_list_foreach(input, &player->destructor.inputs, node)
{
+ fprintf(stderr, "--- DESTRUCTOR THREAD: STOPPING\n");
+
vlc_player_input_HandleState(input, VLC_PLAYER_STATE_STOPPING,
VLC_TICK_INVALID);
vlc_player_destructor_AddStoppingInput(player, input);
input_Stop(input->thread);
+
}
bool keep_sout = true;
@@ -235,6 +244,8 @@ vlc_player_destructor_Thread(void *data)
!vlc_list_is_empty(&player->destructor.joinable_inputs);
vlc_list_foreach(input, &player->destructor.joinable_inputs, node)
{
+ fprintf(stderr, "--- DESTRUCTOR THREAD: JOINABLE\n");
+
vlc_player_UpdateMLStates(player, input);
keep_sout = var_GetBool(input->thread, "sout-keep");
@@ -1041,6 +1052,7 @@ vlc_player_SetCurrentMedia(vlc_player_t *player, input_item_t *media)
if (player->input)
{
+ fprintf(stderr, "--- PLAYER SET MEDIA: ADD INPUT\n");
vlc_player_destructor_AddInput(player, player->input);
player->input = NULL;
}
@@ -1211,6 +1223,7 @@ vlc_player_Stop(vlc_player_t *player)
return VLC_EGENERIC;
player->started = false;
+ fprintf(stderr, "--- PLAYER STOP: ADD INPUT\n");
vlc_player_destructor_AddInput(player, input);
player->input = NULL;
return VLC_SUCCESS;
@@ -1891,6 +1904,7 @@ vlc_player_Delete(vlc_player_t *player)
{
vlc_mutex_lock(&player->lock);
+ fprintf(stderr, "--- PLAYER DELETE: ADD INPUT\n");
if (player->input)
vlc_player_destructor_AddInput(player, player->input)Which ends up with the failing interleaving
--- PLAYER SET MEDIA: ADD INPUT
--- ADDING INPUT TO DESTRUCTOR in INPUTS
--- INPUT THREAD: EVENT DEAD
--- ADDING INPUT TO DESTRUCTOR in JOINABLES