From: Simon Marchi Date: Thu, 30 Jun 2022 14:46:53 +0000 (-0400) Subject: Fix: bt2: autodisc: remove thread error while inserting status in map X-Git-Url: http://git.efficios.com/?p=babeltrace.git;a=commitdiff_plain;h=827e42e017fc5f525aa39a3851bf2e7e50e887aa Fix: bt2: autodisc: remove thread error while inserting status in map If something fails in `bt_bt2_auto_discover_source_components`, we append an error cause and we go to the error label. This function returns a bt_value map containing the return status code and the auto-discovery results, if it was successful. So we then run into: if (result) { insert_entry_status = bt_value_map_insert_signed_integer_entry(result, "status", status); if (insert_entry_status != BT_VALUE_MAP_INSERT_ENTRY_STATUS_OK) { BT_VALUE_PUT_REF_AND_RESET(result); PyErr_NoMemory(); } } However, since there is an error on the current thread, we fail this precondition: 06-30 11:27:11.697 3948806 3948806 F LIB/ASSERT-COND bt_lib_assert_cond_failed@assert-cond.c:64 Babeltrace 2 library precondition not satisfied. 06-30 11:27:11.697 3948806 3948806 F LIB/ASSERT-COND bt_lib_assert_cond_failed@assert-cond.c:66 ------------------------------------------------------------------------ 06-30 11:27:11.697 3948806 3948806 F LIB/ASSERT-COND bt_lib_assert_cond_failed@assert-cond.c:67 Condition ID: `pre:value-map-insert-signed-integer-entry:no-error`. 06-30 11:27:11.697 3948806 3948806 F LIB/ASSERT-COND bt_lib_assert_cond_failed@assert-cond.c:69 Function: bt_value_map_insert_signed_integer_entry(). 06-30 11:27:11.697 3948806 3948806 F LIB/ASSERT-COND bt_lib_assert_cond_failed@assert-cond.c:70 ------------------------------------------------------------------------ 06-30 11:27:11.697 3948806 3948806 F LIB/ASSERT-COND bt_lib_assert_cond_failed@assert-cond.c:71 Error is: 06-30 11:27:11.697 3948806 3948806 F LIB/ASSERT-COND bt_lib_assert_cond_failed@assert-cond.c:73 API function called while current thread has an error: function=bt_value_map_insert_signed_integer_entry 06-30 11:27:11.697 3948806 3948806 F LIB/ASSERT-COND bt_lib_assert_cond_failed@assert-cond.c:76 Aborting... Change the function to temporarily remove the current thread error, while inserting the status in the map. In the unlikely event where the insertion fail because of a memory error, then we just release the error and it gets lost. Change-Id: I1b54fb7c8cb0f719fee867e7385dd6a3949cbde4 Signed-off-by: Simon Marchi Reviewed-on: https://review.lttng.org/c/babeltrace/+/8512 Reviewed-by: Philippe Proulx --- diff --git a/src/bindings/python/bt2/bt2/native_bt_autodisc.i.h b/src/bindings/python/bt2/bt2/native_bt_autodisc.i.h index 43e9dc82..d6a4d24b 100644 --- a/src/bindings/python/bt2/bt2/native_bt_autodisc.i.h +++ b/src/bindings/python/bt2/bt2/native_bt_autodisc.i.h @@ -43,6 +43,7 @@ bt_value *bt_bt2_auto_discover_source_components(const bt_value *inputs, bt_value *components_list = NULL; bt_value *component_info = NULL; bt_value_map_insert_entry_status insert_entry_status; + const bt_error *error = NULL; BT_ASSERT(bt_value_get_type(inputs) == BT_VALUE_TYPE_ARRAY); for (i = 0; i < bt_value_array_get_length(inputs); i++) { @@ -178,11 +179,23 @@ error: end: if (result) { + /* + * If an error happened, we must clear the error temporarily + * while we insert the status in the map. + */ + error = bt_current_thread_take_error(); insert_entry_status = bt_value_map_insert_signed_integer_entry(result, "status", status); - if (insert_entry_status != BT_VALUE_MAP_INSERT_ENTRY_STATUS_OK) { + if (insert_entry_status == BT_VALUE_MAP_INSERT_ENTRY_STATUS_OK) { + if (error) { + bt_current_thread_move_error(error); + error = NULL; + } + } else { BT_VALUE_PUT_REF_AND_RESET(result); PyErr_NoMemory(); } + + } auto_source_discovery_fini(&auto_disc); @@ -190,5 +203,9 @@ end: bt_value_put_ref(components_list); bt_value_put_ref(component_info); + if (error) { + bt_error_release(error); + } + return result; } diff --git a/tests/bindings/python/bt2/test_trace_collection_message_iterator.py b/tests/bindings/python/bt2/test_trace_collection_message_iterator.py index c9a6ebd7..954b149c 100644 --- a/tests/bindings/python/bt2/test_trace_collection_message_iterator.py +++ b/tests/bindings/python/bt2/test_trace_collection_message_iterator.py @@ -26,6 +26,10 @@ _AUTO_SOURCE_DISCOVERY_PARAMS_LOG_LEVEL_PATH = os.path.join( _BT_TESTS_DATADIR, "auto-source-discovery", "params-log-level" ) +_METADATA_SYNTAX_ERROR_TRACE_PATH = os.path.join( + _BT_CTF_TRACES_PATH, "fail", "metadata-syntax-error" +) + class _SomeSource( bt2._UserSourceComponent, message_iterator_class=bt2._UserMessageIterator @@ -709,5 +713,15 @@ class TestAutoDiscoverSourceComponentSpecsParamsObjLogLevel( self.assertEqual(msgs[1].stream.name, "TestSourceB: deore") +class TestAutoDiscoverFailures(unittest.TestCase): + def test_metadata_syntax_error(self): + with self.assertRaisesRegex( + bt2._Error, + 'At line 3 in metadata stream: syntax error, unexpected CTF_RSBRAC: token="]"', + ): + specs = [bt2.AutoSourceComponentSpec(_METADATA_SYNTAX_ERROR_TRACE_PATH)] + bt2.TraceCollectionMessageIterator(specs) + + if __name__ == "__main__": unittest.main()