From bcf95cc0876c7f8e0025b0fb742170acd0959a0e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alicia=20Boya=20Garc=C3=ADa?= Date: Wed, 21 Jun 2023 16:30:52 +0200 Subject: [PATCH] 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 https://gitlab.gnome.org/GNOME/glib/-/commit/8aeca4fa647bfd0f35c4a86b1e6ca6e955519ca5) 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: --- .../tests/check/nle/nlesource.c | 9 +++++- .../tests/check/nle/seek.c | 28 ++++++++++++++++++- .../tests/check/nle/tempochange.c | 23 ++++++++++++--- 3 files changed, 54 insertions(+), 6 deletions(-) diff --git a/subprojects/gst-editing-services/tests/check/nle/nlesource.c b/subprojects/gst-editing-services/tests/check/nle/nlesource.c index 3782b7cbc4..51b0d44e69 100644 --- a/subprojects/gst-editing-services/tests/check/nle/nlesource.c +++ b/subprojects/gst-editing-services/tests/check/nle/nlesource.c @@ -10,6 +10,8 @@ GST_START_TEST (test_simple_videotestsrc) gboolean carry_on = TRUE; GstPad *sinkpad; + ges_init (); + pipeline = gst_pipeline_new ("test_pipeline"); /* @@ -98,6 +100,8 @@ GST_START_TEST (test_simple_videotestsrc) gst_object_unref (bus); g_free (collect); + + ges_deinit (); } GST_END_TEST; @@ -112,6 +116,8 @@ GST_START_TEST (test_videotestsrc_in_bin) gboolean carry_on = TRUE; GstPad *sinkpad; + ges_init (); + pipeline = gst_pipeline_new ("test_pipeline"); /* @@ -198,6 +204,8 @@ GST_START_TEST (test_videotestsrc_in_bin) gst_object_unref (bus); g_free (collect); + + ges_deinit (); } GST_END_TEST; @@ -208,7 +216,6 @@ gnonlin_suite (void) Suite *s = suite_create ("nlesource"); TCase *tc_chain = tcase_create ("nlesource"); - ges_init (); suite_add_tcase (s, tc_chain); if (0) diff --git a/subprojects/gst-editing-services/tests/check/nle/seek.c b/subprojects/gst-editing-services/tests/check/nle/seek.c index 84c460c853..e221d81b2f 100644 --- a/subprojects/gst-editing-services/tests/check/nle/seek.c +++ b/subprojects/gst-editing-services/tests/check/nle/seek.c @@ -1,6 +1,8 @@ #include "common.h" static const gchar *compositor_element = NULL; +static void late_ges_init (void); + typedef struct _SeekInfo { GstClockTime position; /* Seek value and segment position */ @@ -476,6 +478,8 @@ GST_START_TEST (test_complex_operations) GstElement *comp, *oper, *source1, *source2; GList *segments = NULL, *seeks = NULL; + late_ges_init (); + comp = 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; GList *segments = NULL, *seeks = NULL; + late_ges_init (); + comp = gst_element_factory_make_or_warn ("nlecomposition", "test_composition"); @@ -715,6 +721,8 @@ GST_END_TEST; GST_START_TEST (test_simplest) { + late_ges_init (); + test_simplest_full (); } @@ -723,6 +731,8 @@ GST_END_TEST; GST_START_TEST (test_one_after_other) { + late_ges_init (); + test_one_after_other_full (); } @@ -731,6 +741,8 @@ GST_END_TEST; GST_START_TEST (test_one_under_another) { + late_ges_init (); + test_one_under_another_full (); } @@ -739,11 +751,26 @@ GST_END_TEST; GST_START_TEST (test_one_bin_after_other) { + late_ges_init (); + test_one_bin_after_other_full (); } 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 * gnonlin_suite (void) @@ -751,7 +778,6 @@ gnonlin_suite (void) Suite *s = suite_create ("gnonlin-seek"); TCase *tc_chain = tcase_create ("general"); - ges_init (); suite_add_tcase (s, tc_chain); if (gst_registry_check_feature_version (gst_registry_get (), "compositor", 1, diff --git a/subprojects/gst-editing-services/tests/check/nle/tempochange.c b/subprojects/gst-editing-services/tests/check/nle/tempochange.c index 0454f5cc26..d9226fc10b 100644 --- a/subprojects/gst-editing-services/tests/check/nle/tempochange.c +++ b/subprojects/gst-editing-services/tests/check/nle/tempochange.c @@ -20,6 +20,8 @@ #include "common.h" #include "plugins/nle/nleobject.h" +static void late_ges_init (void); + typedef struct _PadEventData { gchar *name; @@ -386,6 +388,8 @@ GST_START_TEST (test_tempochange_play) gdouble rates[3] = { 0.5, 4.0, 1.0 }; guint i, j; + late_ges_init (); + for (i = 0; i < G_N_ELEMENTS (rates); i++) { gdouble rate = rates[i]; GST_DEBUG ("rate = %g", rate); @@ -517,6 +521,8 @@ GST_START_TEST (test_tempochange_seek) guint i, j; GstClockTime offset = 0.1 * GST_SECOND; + late_ges_init (); + for (i = 0; i < G_N_ELEMENTS (rates); i++) { gdouble rate = rates[i]; GST_DEBUG ("rate = %g", rate); @@ -629,17 +635,26 @@ GST_START_TEST (test_tempochange_seek) GST_END_TEST; -static Suite * -gnonlin_suite (void) +static void +late_ges_init () { - Suite *s = suite_create ("nle"); - TCase *tc_chain = tcase_create ("tempochange"); + /* 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 * +gnonlin_suite (void) +{ + Suite *s = suite_create ("nle"); + TCase *tc_chain = tcase_create ("tempochange"); + suite_add_tcase (s, tc_chain); /* give the tests a little more time than the default