From 328e7626745f87f7f298728bd6f21f0645f4f08e Mon Sep 17 00:00:00 2001 From: MyungJoo Ham Date: Sat, 5 Sep 2026 09:44:55 +0900 Subject: [PATCH 1/2] [C-Api] fix self-deadlock on single-shot invoke timeout When an invoke times out, ml_single_invoke() registers the abandoned output in destroy_data_list and, for a framework that allocates the output in invoke, sets its destroy callback to ml_single_destroy_notify_cb(). The invoke thread later releases that output with ml_tensors_data_destroy() while holding single_h->mutex, so the callback re-enters ML_SINGLE_GET_VALID_HANDLE_LOCKED() and waits for the non-recursive mutex it already holds. That wait happens with the global magic lock taken, so every single-shot API in the process blocks. Three call sites release such an output under the mutex, and they do not all fail the same way. __process_output() deadlocks as described. The status != ML_ERROR_NONE || JOIN_REQUESTED branch deadlocks when an invoke fails after a timeout with no close pending, but merely leaks when it is a close that woke it, because ml_single_close() has zeroed magic by then and the callback returns ML_ERROR_INVALID_PARAMETER before reaching the mutex, which makes _ml_tensors_data_destroy_internal() bail out and drop the handle. The exit: label, reached only on JOIN_REQUESTED, leaks for the same reason. Release such an output through __destroy_notify() first and let ml_tensors_data_destroy() free the handle afterwards. The destroy callback is already cleared by then, so the handle mutex is taken once and the framework buffers go back to the framework on every path. __destroy_notify() now also clears the tensor pointers and sizes it has handed back, so nothing frees or copies into them again. This no longer relies on the tensor-filter vtable clearing them on our behalf, which its contract does not promise. Where the filter is already gone, the destroy callback says the same thing, so the descriptor is cleared rather than left for a plain g_free(). The only observable change is that such an output reports a NULL buffer of size 0 after ml_single_close(), where it previously reported the original size with a NULL pointer, so say that in the header next to the close-before-destroy note. Related to #690 (items H1 and H2) Signed-off-by: MyungJoo Ham Co-Authored-By: Claude Opus 5 --- c/include/nnstreamer-single.h | 2 ++ c/src/ml-api-inference-single.c | 57 +++++++++++++++++++++++++-------- 2 files changed, 45 insertions(+), 14 deletions(-) diff --git a/c/include/nnstreamer-single.h b/c/include/nnstreamer-single.h index 07e9c4e8..9c5c036f 100644 --- a/c/include/nnstreamer-single.h +++ b/c/include/nnstreamer-single.h @@ -116,6 +116,8 @@ int ml_single_open_full (ml_single_h *single, const char *model, const ml_tensor * @details Note that this should be called before destroying the inference data by ml_tensors_data_destroy(). * If not, the inference engine might try to access the data that is already freed. * And it causes the segmentation fault. + * An output buffer allocated by the neural network framework is handed back to it + * while closing, so such an output reports a NULL buffer of size 0 afterwards. * @since_tizen 5.5 * @param[in] single The model handle to be closed. * @return @c 0 on success. Otherwise a negative error value. diff --git a/c/src/ml-api-inference-single.c b/c/src/ml-api-inference-single.c index d31b8840..f4ac6958 100644 --- a/c/src/ml-api-inference-single.c +++ b/c/src/ml-api-inference-single.c @@ -361,13 +361,28 @@ __destroy_notify (gpointer data_h, gpointer single_data) { ml_single *single_h; ml_tensors_data_s *data; + gboolean fw_allocated; data = (ml_tensors_data_s *) data_h; single_h = (ml_single *) single_data; + /* the destroy callback is set only for the data allocated by the framework */ + fw_allocated = (data->destroy != NULL); + if (G_LIKELY (single_h->filter)) { - if (single_h->klass->allocate_in_invoke (single_h->filter)) { + fw_allocated = single_h->klass->allocate_in_invoke (single_h->filter); + + if (fw_allocated) single_h->klass->destroy_notify (single_h->filter, data->tensors); + } + + if (fw_allocated) { + guint i; + + /* the buffers belong to the framework, they must not be freed again */ + for (i = 0; i < data->num_tensors; i++) { + data->tensors[i].data = NULL; + data->tensors[i].size = 0; } } @@ -375,6 +390,28 @@ __destroy_notify (gpointer data_h, gpointer single_data) data->destroy = NULL; } +/** + * @brief Releases the output data which may be registered in the destroy list. + * @note Do not call ml_tensors_data_destroy() on such data while holding + * single_h->mutex; its destroy callback takes the same mutex again. + */ +static void +__release_output_data (ml_single * single_h, ml_tensors_data_h output) +{ + ml_tensors_data_s *data = (ml_tensors_data_s *) output; + + if (!data) + return; + + single_h->destroy_data_list = + g_list_remove (single_h->destroy_data_list, output); + + if (data->destroy) + __destroy_notify (data, single_h); + + ml_tensors_data_destroy (output); +} + /** * @brief Wrapper function for __destroy_notify */ @@ -480,9 +517,7 @@ __process_output (ml_single * single_h, ml_tensors_data_h output) * Caller of the invoke thread has returned back with timeout. * So, free the memory allocated by the invoke as their is no receiver. */ - single_h->destroy_data_list = - g_list_remove (single_h->destroy_data_list, output); - ml_tensors_data_destroy (output); + __release_output_data (single_h, output); } else { out_data = (ml_tensors_data_s *) output; set_destroy_notify (single_h, out_data, FALSE); @@ -544,11 +579,8 @@ invoke_thread (void *arg) single_h->invoking = FALSE; if (status != ML_ERROR_NONE || single_h->state == JOIN_REQUESTED) { - if (alloc_output) { - single_h->destroy_data_list = - g_list_remove (single_h->destroy_data_list, output); - ml_tensors_data_destroy (output); - } + if (alloc_output) + __release_output_data (single_h, output); if (single_h->state == JOIN_REQUESTED) goto exit; @@ -573,11 +605,8 @@ invoke_thread (void *arg) if (single_h->input) ml_tensors_data_destroy (single_h->input); - if (alloc_output && single_h->output) { - single_h->destroy_data_list = - g_list_remove (single_h->destroy_data_list, single_h->output); - ml_tensors_data_destroy (single_h->output); - } + if (alloc_output && single_h->output) + __release_output_data (single_h, single_h->output); single_h->input = single_h->output = NULL; g_cond_broadcast (&single_h->cond); From 608a2aa030d3bc71bda6e49bbc0c1f309e8db258 Mon Sep 17 00:00:00 2001 From: MyungJoo Ham Date: Sat, 5 Sep 2026 09:45:11 +0900 Subject: [PATCH 2/2] [Test] cover the single-shot invoke timeout in CI The custom filter these tests would naturally use, libnnstreamer_customfilter_scaler_allocator, ships only in nnstreamer's unittests RPM and in no Debian package, and neither packaging/machine-learning-api.spec nor debian/control pulls it in. The existing invoke_10_p, invoke_11_p and invoke_12_p have been skipping silently for that reason, each reporting 0 ms. A regression in the timeout path also shows up as a hang, and packaging/run_unittests.sh starts the gtest binaries with no timeout, so it would stall the job rather than fail it. Build a custom filter for the test suite itself. It allocates the output in invoke, so the single-shot handle takes the allocate_in_invoke path, and each invoke sleeps long enough for a short ml_single_set_timeout() to fire regardless of how fast the machine is. It is found through MLAPI_BUILD_ROOT_PATH, which both debian/rules and the spec %check export, with fallbacks for the build directory that run_unittests.sh leaves as the cwd and for the installed layout that run-unittest.sh uses. Since it is built alongside the test binary, not finding it means the setup is broken, so the tests assert on it rather than skip. Guard the timeout test with a watchdog thread that ends the process after 30 seconds. A self-deadlock parks the invoke thread while it holds the global handle lock, so the run cannot continue anyway; failing right away turns a stalled job into a normal test failure. invoke_timeout_alloc_in_invoke_p times out twice, once with the handle open and once with a close pending, to reach the first two release sites. close_before_data_destroy_01_p checks the close-before-destroy order documented in nnstreamer-single.h on an allocating framework, and close_before_data_destroy_02_p checks that a tensorflow-lite output, which the handle owns, is untouched by the same close. Related to #690 (items H1 and H2) Signed-off-by: MyungJoo Ham Co-Authored-By: Claude Opus 5 --- debian/ml-api-unittests.install | 1 + tests/capi/meson.build | 8 + .../capi/ml_api_customfilter_slow_allocator.c | 113 +++++++ tests/capi/unittest_capi_inference_single.cc | 295 ++++++++++++++++++ 4 files changed, 417 insertions(+) create mode 100644 tests/capi/ml_api_customfilter_slow_allocator.c diff --git a/debian/ml-api-unittests.install b/debian/ml-api-unittests.install index 924c951c..97e72d1e 100644 --- a/debian/ml-api-unittests.install +++ b/debian/ml-api-unittests.install @@ -1,2 +1,3 @@ /usr/lib/nnstreamer/bin/unittest-ml/tests/unittest_capi* +/usr/lib/nnstreamer/bin/unittest-ml/tests/libml_api_customfilter* /usr/lib/nnstreamer/bin/unittest-ml/tests/test_models diff --git a/tests/capi/meson.build b/tests/capi/meson.build index 60223d83..7dae76b0 100644 --- a/tests/capi/meson.build +++ b/tests/capi/meson.build @@ -1,3 +1,11 @@ +# Custom filter allocating the output in invoke, used by the single-shot tests. +shared_library('ml_api_customfilter_slow_allocator', + 'ml_api_customfilter_slow_allocator.c', + dependencies: [glib_dep, gst_dep, nnstreamer_dep], + install: get_option('install-test'), + install_dir: unittest_install_dir +) + unittest_capi_inference_single = executable('unittest_capi_inference_single', 'unittest_capi_inference_single.cc', dependencies: [nns_capi_single_dep, gtest_dep], diff --git a/tests/capi/ml_api_customfilter_slow_allocator.c b/tests/capi/ml_api_customfilter_slow_allocator.c new file mode 100644 index 00000000..7d6f93a0 --- /dev/null +++ b/tests/capi/ml_api_customfilter_slow_allocator.c @@ -0,0 +1,113 @@ +/** + * SPDX-License-Identifier: Apache-2.0 + */ +/** + * @file ml_api_customfilter_slow_allocator.c + * @date 4 Sep 2026 + * @brief Custom filter for the ML API unittests. + * @see https://github.com/nnstreamer/api + * @author MyungJoo Ham + * @bug No known bugs + * + * This copies the input into a buffer that the filter itself allocates, so a + * single-shot handle using it takes the "allocate_in_invoke" path. Each invoke + * deliberately takes longer than a short ml_single_set_timeout(), which lets + * the unittests reach the timeout handling without depending on the machine. + */ + +#include +#include +#include +#include +#include + +#define INVOKE_DELAY_USEC (200000U) + +/** + * @brief init callback of tensor_filter custom + */ +static void * +pt_init (const GstTensorFilterProperties * prop) +{ + UNUSED (prop); + return g_new0 (guint, 1); +} + +/** + * @brief exit callback of tensor_filter custom + */ +static void +pt_exit (void *private_data, const GstTensorFilterProperties * prop) +{ + UNUSED (prop); + g_free (private_data); +} + +/** + * @brief setInputDimension callback of tensor_filter custom + */ +static int +set_inputDim (void *private_data, const GstTensorFilterProperties * prop, + const GstTensorsInfo * in_info, GstTensorsInfo * out_info) +{ + UNUSED (private_data); + UNUSED (prop); + + gst_tensors_info_copy (out_info, in_info); + return 0; +} + +/** + * @brief allocate-invoke callback of tensor_filter custom + */ +static int +pt_allocate_invoke (void *private_data, const GstTensorFilterProperties * prop, + const GstTensorMemory * input, GstTensorMemory * output) +{ + GstTensorsInfo *out_meta, *in_meta; + guint i; + UNUSED (private_data); + + if (prop->input_meta.num_tensors != prop->output_meta.num_tensors) + return -1; + + out_meta = (GstTensorsInfo *) & prop->output_meta; + in_meta = (GstTensorsInfo *) & prop->input_meta; + + g_usleep (INVOKE_DELAY_USEC); + + for (i = 0; i < out_meta->num_tensors; i++) { + GstTensorInfo *_out = gst_tensors_info_get_nth_info (out_meta, i); + GstTensorInfo *_in = gst_tensors_info_get_nth_info (in_meta, i); + gsize size = gst_tensor_info_get_size (_out); + gsize in_size = gst_tensor_info_get_size (_in); + + output[i].data = g_malloc (size); + memcpy (output[i].data, input[i].data, MIN (size, in_size)); + } + + return 0; +} + +/** + * @brief destroy-notify callback of tensor_filter custom + */ +static void +pt_destroy_notify (void *data) +{ + g_free (data); +} + +/** + * @brief tensor_filter custom subplugin definition + */ +static NNStreamer_custom_class NNStreamer_custom_body = { + .initfunc = pt_init, + .exitfunc = pt_exit, + .setInputDim = set_inputDim, + .allocate_invoke = pt_allocate_invoke, + .destroy_notify = pt_destroy_notify, +}; + +/* The dyn-loaded object */ +NNStreamer_custom_class *NNStreamer_custom = &NNStreamer_custom_body; diff --git a/tests/capi/unittest_capi_inference_single.cc b/tests/capi/unittest_capi_inference_single.cc index c30f4b76..fba763b0 100644 --- a/tests/capi/unittest_capi_inference_single.cc +++ b/tests/capi/unittest_capi_inference_single.cc @@ -16,6 +16,7 @@ #include #include #include +#include #if defined(__APPLE__) #define SO_FILE_EXTENSION ".dylib" @@ -3440,6 +3441,300 @@ TEST (nnstreamer_capi_singleshot, invoke_12_p) g_free (test_model); } +/** + * @brief Locate the custom filter that this test suite builds for itself. + * @details It sits next to this binary once installed, and under tests/capi in + * a build tree. It is built along with this binary, so failing to find + * it means the test setup is broken and the caller should not go on. + * @return Newly allocated path to the shared object, NULL if it is not found. + */ +static gchar * +_get_test_custom_filter (void) +{ + const gchar cf_name[] = "libml_api_customfilter_slow_allocator" SO_FILE_EXTENSION; + const gchar *build_root = g_getenv ("MLAPI_BUILD_ROOT_PATH"); + gchar *exe, *dir, *path; + + if (build_root != NULL) { + path = g_build_filename (build_root, "tests", "capi", cf_name, NULL); + if (g_file_test (path, G_FILE_TEST_EXISTS)) + return path; + g_free (path); + } + + /* the test runner starts the binaries from the build directory */ + path = g_build_filename ("tests", "capi", cf_name, NULL); + if (g_file_test (path, G_FILE_TEST_EXISTS)) + return path; + g_free (path); + + exe = g_file_read_link ("/proc/self/exe", NULL); + if (exe != NULL) { + dir = g_path_get_dirname (exe); + g_free (exe); + + path = g_build_filename (dir, cf_name, NULL); + g_free (dir); + + if (g_file_test (path, G_FILE_TEST_EXISTS)) + return path; + g_free (path); + } + + return NULL; +} + +/** + * @brief Aborts the process when the single-shot API stops responding. + * @details A self-deadlock in the invoke thread cannot be recovered from. It + * parks while holding the global handle lock, so the test that hit it + * and every test after it would hang until the CI job is killed. + */ +static gpointer +_singleshot_watchdog (gpointer user_data) +{ + gint *done = (gint *) user_data; + gint64 end_time = g_get_monotonic_time () + 30 * G_TIME_SPAN_SECOND; + + while (g_get_monotonic_time () < end_time) { + if (g_atomic_int_get (done)) + return NULL; + g_usleep (100000U); + } + + g_printerr ("The single-shot invoke thread did not release the timed out " + "output within 30 seconds; the handle lock is deadlocked.\n"); + _exit (1); + + return NULL; +} + +/** + * @brief Test NNStreamer single shot (custom filter) + * @detail Destroy the output data after closing the handle, which is the order + * described in nnstreamer-single.h. The buffers taken back by the + * framework should not be reachable nor freed again. + */ +TEST (nnstreamer_capi_singleshot, close_before_data_destroy_01_p) +{ + gchar *test_model = NULL; + ml_single_h single; + ml_tensors_info_h in_info, out_info; + ml_tensors_data_h input, output; + ml_tensor_dimension in_dim; + int status; + unsigned int i; + int16_t dummy = 0; + void *data_ptr; + size_t data_size; + + test_model = _get_test_custom_filter (); + ASSERT_TRUE (test_model != NULL); + + ml_tensors_info_create (&in_info); + ml_tensors_info_create (&out_info); + + ml_tensors_info_set_count (in_info, 1); + + in_dim[0] = 10; + in_dim[1] = 1; + in_dim[2] = 1; + in_dim[3] = 1; + + ml_tensors_info_set_tensor_type (in_info, 0, ML_TENSOR_TYPE_INT16); + ml_tensors_info_set_tensor_dimension (in_info, 0, in_dim); + + ml_tensors_info_clone (out_info, in_info); + + status = ml_single_open (&single, test_model, in_info, out_info, + ML_NNFW_TYPE_CUSTOM_FILTER, ML_NNFW_HW_ANY); + ASSERT_EQ (status, ML_ERROR_NONE); + + input = output = NULL; + + /* generate input data */ + status = ml_tensors_data_create (in_info, &input); + EXPECT_EQ (status, ML_ERROR_NONE); + ASSERT_TRUE (input != NULL); + + status = ml_tensors_data_get_tensor_data (input, 0, &data_ptr, &data_size); + EXPECT_EQ (status, ML_ERROR_NONE); + for (i = 0; i < 10; i++) { + ((int16_t *) data_ptr)[i] = (int16_t) (i + 1); + } + + status = ml_single_invoke (single, input, &output); + EXPECT_EQ (status, ML_ERROR_NONE); + ASSERT_TRUE (output != NULL); + + status = ml_single_close (single); + EXPECT_EQ (status, ML_ERROR_NONE); + + /* the framework has taken its buffers back while closing the handle */ + status = ml_tensors_data_get_tensor_data (output, 0, &data_ptr, &data_size); + EXPECT_EQ (status, ML_ERROR_NONE); + EXPECT_TRUE (data_ptr == NULL); + EXPECT_EQ (data_size, 0U); + + status = ml_tensors_data_set_tensor_data (output, 0, &dummy, sizeof (dummy)); + EXPECT_NE (status, ML_ERROR_NONE); + + status = ml_tensors_data_destroy (output); + EXPECT_EQ (status, ML_ERROR_NONE); + + status = ml_tensors_data_destroy (input); + EXPECT_EQ (status, ML_ERROR_NONE); + + ml_tensors_info_destroy (in_info); + ml_tensors_info_destroy (out_info); + g_free (test_model); +} + +/** + * @brief Test NNStreamer single shot (tensorflow-lite) + * @detail The output buffer of a framework that does not allocate in invoke + * belongs to the data handle and stays valid after closing the handle. + */ +TEST (nnstreamer_capi_singleshot, close_before_data_destroy_02_p) +{ + ml_single_h single; + ml_tensors_info_h in_info = NULL; + ml_tensors_data_h input, output; + int status; + void *data_ptr; + size_t data_size; + + const gchar *root_path = g_getenv ("MLAPI_SOURCE_ROOT_PATH"); + gchar *test_model; + + /* supposed to run test in build directory */ + if (root_path == NULL) + root_path = ".."; + + /* add.tflite adds value 2 to all the values in the input */ + test_model = g_build_filename ( + root_path, "tests", "test_models", "models", "add.tflite", NULL); + ASSERT_TRUE (g_file_test (test_model, G_FILE_TEST_EXISTS)); + + status = ml_single_open (&single, test_model, NULL, NULL, + ML_NNFW_TYPE_TENSORFLOW_LITE, ML_NNFW_HW_ANY); + if (is_enabled_tensorflow_lite) { + EXPECT_EQ (status, ML_ERROR_NONE); + } else { + EXPECT_NE (status, ML_ERROR_NONE); + goto skip_test; + } + + status = ml_single_get_input_info (single, &in_info); + EXPECT_EQ (status, ML_ERROR_NONE); + + input = output = NULL; + + status = ml_tensors_data_create (in_info, &input); + EXPECT_EQ (status, ML_ERROR_NONE); + ASSERT_TRUE (input != NULL); + + status = ml_single_invoke (single, input, &output); + EXPECT_EQ (status, ML_ERROR_NONE); + ASSERT_TRUE (output != NULL); + + status = ml_single_close (single); + EXPECT_EQ (status, ML_ERROR_NONE); + + status = ml_tensors_data_get_tensor_data (output, 0, &data_ptr, &data_size); + EXPECT_EQ (status, ML_ERROR_NONE); + EXPECT_TRUE (data_ptr != NULL); + EXPECT_GT (data_size, 0U); + + status = ml_tensors_data_destroy (output); + EXPECT_EQ (status, ML_ERROR_NONE); + + status = ml_tensors_data_destroy (input); + EXPECT_EQ (status, ML_ERROR_NONE); + + ml_tensors_info_destroy (in_info); + +skip_test: + g_free (test_model); +} + +/** + * @brief Test NNStreamer single shot (custom filter) + * @detail Time out while the framework allocates the output in invoke. The + * invoke thread releases the abandoned output on its own and must not + * take the handle mutex again to do so. The first invoke is released + * while the handle is open, the second one while a close is pending. + */ +TEST (nnstreamer_capi_singleshot, invoke_timeout_alloc_in_invoke_p) +{ + gchar *test_model = NULL; + ml_single_h single; + ml_tensors_info_h in_info, out_info; + ml_tensors_data_h input, output; + ml_tensor_dimension in_dim; + GThread *watchdog; + gint done = 0; + int status; + + test_model = _get_test_custom_filter (); + ASSERT_TRUE (test_model != NULL); + + ml_tensors_info_create (&in_info); + ml_tensors_info_create (&out_info); + + ml_tensors_info_set_count (in_info, 1); + + in_dim[0] = 10; + in_dim[1] = 1; + in_dim[2] = 1; + in_dim[3] = 1; + + ml_tensors_info_set_tensor_type (in_info, 0, ML_TENSOR_TYPE_INT16); + ml_tensors_info_set_tensor_dimension (in_info, 0, in_dim); + + ml_tensors_info_clone (out_info, in_info); + + status = ml_single_open (&single, test_model, in_info, out_info, + ML_NNFW_TYPE_CUSTOM_FILTER, ML_NNFW_HW_ANY); + ASSERT_EQ (status, ML_ERROR_NONE); + + input = output = NULL; + + status = ml_tensors_data_create (in_info, &input); + EXPECT_EQ (status, ML_ERROR_NONE); + ASSERT_TRUE (input != NULL); + + watchdog = g_thread_new ("singleshot-watchdog", _singleshot_watchdog, &done); + + status = ml_single_set_timeout (single, 10); + if (status == ML_ERROR_NONE) { + /* the invoke thread drops the abandoned output while the handle is open */ + status = ml_single_invoke (single, input, &output); + EXPECT_EQ (status, ML_ERROR_TIMED_OUT); + EXPECT_TRUE (output == NULL); + + g_usleep (500000U); + + /* and drops it again while the close is pending */ + status = ml_single_invoke (single, input, &output); + EXPECT_TRUE (status == ML_ERROR_TIMED_OUT || status == ML_ERROR_TRY_AGAIN); + EXPECT_TRUE (output == NULL); + } + + status = ml_single_close (single); + EXPECT_EQ (status, ML_ERROR_NONE); + + g_atomic_int_set (&done, 1); + g_thread_join (watchdog); + + status = ml_tensors_data_destroy (input); + EXPECT_EQ (status, ML_ERROR_NONE); + + ml_tensors_info_destroy (in_info); + ml_tensors_info_destroy (out_info); + g_free (test_model); +} + /** * @brief Test NNStreamer single shot (custom filter) * @detail Change the number of input tensors, run the model and verify output