diff options
author | Hanno de Wind <hannodewind@gmail.com> | 2024-02-19 10:24:14 +0000 |
---|---|---|
committer | joergho <48011876+joergho@users.noreply.github.com> | 2024-04-29 09:24:42 +0200 |
commit | 0f2007fc97b58699173763c1f3ed7c7ee7b7f854 (patch) | |
tree | a9c1347bed73085323f849c96d1a899d75844583 | |
parent | rfnoc: fir filter: Enable newer revisions (diff) | |
download | uhd-master.tar.xz uhd-master.zip |
Processing of the action queue gets locked up when any action
being executed in the send_action call throws an exception.
Exceptions are not caught in the loop handling the action queue,
resulting in the handling_ongoing queue locking flag to never
be released. Any subsequent call to enqueue_action will return
on the early exit with the assumption that we're already handling
the actions, yet the previous handler exited with an exception.
This fix uses scope_exit rather than a manually claimed and
released atomic flag to ensure that the handling_ongoing will be
released even under exceptional conditions.
Co-authored-by: Martin Braun <martin.braun@ettus.com>
-rw-r--r-- | host/lib/rfnoc/graph.cpp | 9 | ||||
-rw-r--r-- | host/tests/actions_test.cpp | 26 |
2 files changed, 31 insertions, 4 deletions
diff --git a/host/lib/rfnoc/graph.cpp b/host/lib/rfnoc/graph.cpp index b84ceee3e..00b0d9a5e 100644 --- a/host/lib/rfnoc/graph.cpp +++ b/host/lib/rfnoc/graph.cpp @@ -6,6 +6,7 @@ #include <uhd/exception.hpp> #include <uhd/utils/log.hpp> +#include <uhd/utils/scope_exit.hpp> #include <uhdlib/rfnoc/graph.hpp> #include <uhdlib/rfnoc/node_accessor.hpp> #include <boost/graph/filtered_graph.hpp> @@ -557,6 +558,8 @@ void graph_t::enqueue_action( << action->id); return; } + auto reset_handling_flag = + uhd::utils::scope_exit::make([&]() { _action_handling_ongoing.clear(); }); unsigned iteration_count = 0; while (!_action_queue.empty()) { @@ -607,10 +610,8 @@ void graph_t::enqueue_action( } UHD_LOG_TRACE(LOG_ID, "Delivered all actions, terminating action handling."); - // Release the action handling flag - _action_handling_ongoing.clear(); - // Now, the _graph_mutex is released, and someone else can start sending - // actions. + // Now, the _graph_mutex and _action_handling_ongoing are released, and + // someone else can start sending actions. } /****************************************************************************** diff --git a/host/tests/actions_test.cpp b/host/tests/actions_test.cpp index d2b5a6e3a..769cbea7a 100644 --- a/host/tests/actions_test.cpp +++ b/host/tests/actions_test.cpp @@ -221,3 +221,29 @@ BOOST_AUTO_TEST_CASE(test_action_forwarding_map_exception_invalid_destination) auto cmd = action_info::make("action"); BOOST_REQUIRE_THROW(generator.post_output_edge_action(cmd, 0), uhd::rfnoc_error); } + +BOOST_AUTO_TEST_CASE(test_action_exception_handling) +{ + node_accessor_t node_accessor{}; + uhd::rfnoc::detail::graph_t graph{}; + + class mock_throwing_node_t : public mock_radio_node_t + { + public: + mock_throwing_node_t() : mock_radio_node_t(0) + { + register_action_handler( + "throwing_action", [](const res_source_info&, action_info::sptr) { + throw uhd::runtime_error("Arbitrary UHD exception"); + }); + } + }; + + mock_throwing_node_t mock_radio{}; + graph.connect(&mock_radio, &mock_radio, {0, 0, graph_edge_t::DYNAMIC, false}); + graph.commit(); + // Check that it throws the first time + BOOST_REQUIRE_THROW(node_accessor.post_action(&mock_radio, {res_source_info::USER, 0}, action_info::make("throwing_action")), uhd::runtime_error); + // It should also throw the second time: we should actually be running this action even though the previous one threw an exception + BOOST_REQUIRE_THROW(node_accessor.post_action(&mock_radio, {res_source_info::USER, 0}, action_info::make("throwing_action")), uhd::runtime_error); +} |