tests: nle: Call ges_init() only inside test cases

Some NLE tests were calling ges_init() from the test suite
initialization function. This was causing them to deadlock when running
with glib 2.76.3.

ges_init() causes a GThreadPool to be initialized. Even if GES at this
point doesn't request any thread to be created, since glib 2.63.4+
(see 8aeca4fa64)
the first time a GThreadPool is initialized a "pool-spawner" thread is
created, which is later used by g_thread_pool_push().

The default behavior of the GStreamer check tests is to fork for every
test case. This is not safe if any thread has been created at this
point. In this particular case, GThreadPool preserves the state that
says a "pool-spawner" thread has been created, and will have access to
its mutex and condition variable, but their queues will have different
contents as the memory has been forked. In consequence, calls to
g_thread_pool_push() will deadlock.

The deadlock will not occur if running the tests with CK_FORK=no.

This patch modifies the affected tests to only call ges_init() from
inside the test cases, fixing the deadlock.

Part-of: <https://gitlab.freedesktop.org/gstreamer/gstreamer/-/merge_requests/4915>
This commit is contained in:
Alicia Boya García 2023-06-21 16:30:52 +02:00 committed by GStreamer Marge Bot
parent 98fbbe7a53
commit bcf95cc087
3 changed files with 54 additions and 6 deletions

View file

@ -10,6 +10,8 @@ GST_START_TEST (test_simple_videotestsrc)
gboolean carry_on = TRUE; gboolean carry_on = TRUE;
GstPad *sinkpad; GstPad *sinkpad;
ges_init ();
pipeline = gst_pipeline_new ("test_pipeline"); pipeline = gst_pipeline_new ("test_pipeline");
/* /*
@ -98,6 +100,8 @@ GST_START_TEST (test_simple_videotestsrc)
gst_object_unref (bus); gst_object_unref (bus);
g_free (collect); g_free (collect);
ges_deinit ();
} }
GST_END_TEST; GST_END_TEST;
@ -112,6 +116,8 @@ GST_START_TEST (test_videotestsrc_in_bin)
gboolean carry_on = TRUE; gboolean carry_on = TRUE;
GstPad *sinkpad; GstPad *sinkpad;
ges_init ();
pipeline = gst_pipeline_new ("test_pipeline"); pipeline = gst_pipeline_new ("test_pipeline");
/* /*
@ -198,6 +204,8 @@ GST_START_TEST (test_videotestsrc_in_bin)
gst_object_unref (bus); gst_object_unref (bus);
g_free (collect); g_free (collect);
ges_deinit ();
} }
GST_END_TEST; GST_END_TEST;
@ -208,7 +216,6 @@ gnonlin_suite (void)
Suite *s = suite_create ("nlesource"); Suite *s = suite_create ("nlesource");
TCase *tc_chain = tcase_create ("nlesource"); TCase *tc_chain = tcase_create ("nlesource");
ges_init ();
suite_add_tcase (s, tc_chain); suite_add_tcase (s, tc_chain);
if (0) if (0)

View file

@ -1,6 +1,8 @@
#include "common.h" #include "common.h"
static const gchar *compositor_element = NULL; static const gchar *compositor_element = NULL;
static void late_ges_init (void);
typedef struct _SeekInfo typedef struct _SeekInfo
{ {
GstClockTime position; /* Seek value and segment position */ GstClockTime position; /* Seek value and segment position */
@ -476,6 +478,8 @@ GST_START_TEST (test_complex_operations)
GstElement *comp, *oper, *source1, *source2; GstElement *comp, *oper, *source1, *source2;
GList *segments = NULL, *seeks = NULL; GList *segments = NULL, *seeks = NULL;
late_ges_init ();
comp = comp =
gst_element_factory_make_or_warn ("nlecomposition", "test_composition"); gst_element_factory_make_or_warn ("nlecomposition", "test_composition");
@ -593,6 +597,8 @@ GST_START_TEST (test_complex_operations_bis)
GstElement *comp, *oper, *source1, *source2; GstElement *comp, *oper, *source1, *source2;
GList *segments = NULL, *seeks = NULL; GList *segments = NULL, *seeks = NULL;
late_ges_init ();
comp = comp =
gst_element_factory_make_or_warn ("nlecomposition", "test_composition"); gst_element_factory_make_or_warn ("nlecomposition", "test_composition");
@ -715,6 +721,8 @@ GST_END_TEST;
GST_START_TEST (test_simplest) GST_START_TEST (test_simplest)
{ {
late_ges_init ();
test_simplest_full (); test_simplest_full ();
} }
@ -723,6 +731,8 @@ GST_END_TEST;
GST_START_TEST (test_one_after_other) GST_START_TEST (test_one_after_other)
{ {
late_ges_init ();
test_one_after_other_full (); test_one_after_other_full ();
} }
@ -731,6 +741,8 @@ GST_END_TEST;
GST_START_TEST (test_one_under_another) GST_START_TEST (test_one_under_another)
{ {
late_ges_init ();
test_one_under_another_full (); test_one_under_another_full ();
} }
@ -739,11 +751,26 @@ GST_END_TEST;
GST_START_TEST (test_one_bin_after_other) GST_START_TEST (test_one_bin_after_other)
{ {
late_ges_init ();
test_one_bin_after_other_full (); test_one_bin_after_other_full ();
} }
GST_END_TEST; GST_END_TEST;
static void
late_ges_init ()
{
/* We need to do this inside the test cases, not during the initialization
* of the suite, as ges_init() will initialize thread pools, which cannot
* work properly after a fork. */
if (atexit (ges_deinit) != 0) {
GST_ERROR ("failed to set ges_deinit as exit function");
}
ges_init ();
}
static Suite * static Suite *
gnonlin_suite (void) gnonlin_suite (void)
@ -751,7 +778,6 @@ gnonlin_suite (void)
Suite *s = suite_create ("gnonlin-seek"); Suite *s = suite_create ("gnonlin-seek");
TCase *tc_chain = tcase_create ("general"); TCase *tc_chain = tcase_create ("general");
ges_init ();
suite_add_tcase (s, tc_chain); suite_add_tcase (s, tc_chain);
if (gst_registry_check_feature_version (gst_registry_get (), "compositor", 1, if (gst_registry_check_feature_version (gst_registry_get (), "compositor", 1,

View file

@ -20,6 +20,8 @@
#include "common.h" #include "common.h"
#include "plugins/nle/nleobject.h" #include "plugins/nle/nleobject.h"
static void late_ges_init (void);
typedef struct _PadEventData typedef struct _PadEventData
{ {
gchar *name; gchar *name;
@ -386,6 +388,8 @@ GST_START_TEST (test_tempochange_play)
gdouble rates[3] = { 0.5, 4.0, 1.0 }; gdouble rates[3] = { 0.5, 4.0, 1.0 };
guint i, j; guint i, j;
late_ges_init ();
for (i = 0; i < G_N_ELEMENTS (rates); i++) { for (i = 0; i < G_N_ELEMENTS (rates); i++) {
gdouble rate = rates[i]; gdouble rate = rates[i];
GST_DEBUG ("rate = %g", rate); GST_DEBUG ("rate = %g", rate);
@ -517,6 +521,8 @@ GST_START_TEST (test_tempochange_seek)
guint i, j; guint i, j;
GstClockTime offset = 0.1 * GST_SECOND; GstClockTime offset = 0.1 * GST_SECOND;
late_ges_init ();
for (i = 0; i < G_N_ELEMENTS (rates); i++) { for (i = 0; i < G_N_ELEMENTS (rates); i++) {
gdouble rate = rates[i]; gdouble rate = rates[i];
GST_DEBUG ("rate = %g", rate); GST_DEBUG ("rate = %g", rate);
@ -629,17 +635,26 @@ GST_START_TEST (test_tempochange_seek)
GST_END_TEST; GST_END_TEST;
static Suite * static void
gnonlin_suite (void) late_ges_init ()
{ {
Suite *s = suite_create ("nle"); /* We need to do this inside the test cases, not during the initialization
TCase *tc_chain = tcase_create ("tempochange"); * of the suite, as ges_init() will initialize thread pools, which cannot
* work properly after a fork. */
if (atexit (ges_deinit) != 0) { if (atexit (ges_deinit) != 0) {
GST_ERROR ("failed to set ges_deinit as exit function"); GST_ERROR ("failed to set ges_deinit as exit function");
} }
ges_init (); ges_init ();
}
static Suite *
gnonlin_suite (void)
{
Suite *s = suite_create ("nle");
TCase *tc_chain = tcase_create ("tempochange");
suite_add_tcase (s, tc_chain); suite_add_tcase (s, tc_chain);
/* give the tests a little more time than the default /* give the tests a little more time than the default