From: Philippe Proulx Date: Tue, 19 Mar 2019 18:51:38 +0000 (-0400) Subject: lib: mark graph as faulty when adding a comp. or connecting ports fails X-Git-Url: http://git.efficios.com/?p=babeltrace.git;a=commitdiff_plain;h=38cda5da795de933c79e8c79b6dcec930c2ce042 lib: mark graph as faulty when adding a comp. or connecting ports fails This patch makes the graph's state become `BT_GRAPH_CONFIGURATION_STATE_FAULTY` when there's a configuration error, that is, when one of the bt_graph_add_*_component*() functions or the bt_graph_connect_ports() fail. When a graph is faulty, it becomes impossible to use it in any way, either to continue configuring it or to run it. This new state makes it invalid to do anything with a graph when it failed one of its configuration steps, so that each step must succeed in order to get a valid, useable, configured graph. This fixes the issue where in bt_graph_connect_ports(), one of the "port connected" method would succeed but the other would fail: in this scenario, the connection ends immediately (disconnecting each port), but one component still believes that its port was successfully connected (as there's no "port disconnected" method, because we don't need it for anything else). With this patch, the component's "port connected" is still called and successful, but the component will never be used anyway, so there's less danger for errors. bt_port_is_connected() will indeed return `BT_FALSE` for this port in the component's finalization method, for example. The alternative would be to add an optional component method to indicate that a port which was just connected is in fact not connected because the other component's "port connected" method failed. This is possible, but what can a graph's user do with a failing bt_graph_connect_ports() anyway? If the intention was to connect two ports, and this operation fails, then there's probably no plan B: retrying the exact same port connection would probably fail again. Signed-off-by: Philippe Proulx --- diff --git a/include/babeltrace/graph/graph-internal.h b/include/babeltrace/graph/graph-internal.h index 97289cf0..83812e7b 100644 --- a/include/babeltrace/graph/graph-internal.h +++ b/include/babeltrace/graph/graph-internal.h @@ -43,6 +43,7 @@ enum bt_graph_configuration_state { BT_GRAPH_CONFIGURATION_STATE_CONFIGURING, BT_GRAPH_CONFIGURATION_STATE_PARTIALLY_CONFIGURED, BT_GRAPH_CONFIGURATION_STATE_CONFIGURED, + BT_GRAPH_CONFIGURATION_STATE_FAULTY, }; struct bt_graph { @@ -211,6 +212,8 @@ enum bt_graph_status bt_graph_configure(struct bt_graph *graph) enum bt_graph_status status = BT_GRAPH_STATUS_OK; uint64_t i; + BT_ASSERT(graph->config_state != BT_GRAPH_CONFIGURATION_STATE_FAULTY); + if (likely(graph->config_state == BT_GRAPH_CONFIGURATION_STATE_CONFIGURED)) { goto end; diff --git a/lib/graph/graph.c b/lib/graph/graph.c index 49933d54..29ebee34 100644 --- a/lib/graph/graph.c +++ b/lib/graph/graph.c @@ -408,7 +408,7 @@ enum bt_graph_status bt_graph_connect_ports( BT_ASSERT_PRE(!graph->canceled, "Graph is canceled: %!+g", graph); BT_ASSERT_PRE( graph->config_state == BT_GRAPH_CONFIGURATION_STATE_CONFIGURING, - "Graph is already configured: %!+g", graph); + "Graph is not in the \"configuring\" state: %!+g", graph); BT_ASSERT_PRE(!bt_port_is_connected(upstream_port), "Upstream port is already connected: %!+p", upstream_port); BT_ASSERT_PRE(!bt_port_is_connected(downstream_port), @@ -543,6 +543,10 @@ enum bt_graph_status bt_graph_connect_ports( } end: + if (status != BT_GRAPH_STATUS_OK) { + graph->config_state = BT_GRAPH_CONFIGURATION_STATE_FAULTY; + } + bt_object_put_ref(connection); (void) init_can_consume; bt_graph_set_can_consume(graph, init_can_consume); @@ -680,6 +684,8 @@ enum bt_graph_status bt_graph_consume(struct bt_graph *graph) BT_ASSERT_PRE(!graph->canceled, "Graph is canceled: %!+g", graph); BT_ASSERT_PRE(graph->can_consume, "Cannot consume graph in its current state: %!+g", graph); + BT_ASSERT_PRE(graph->config_state != BT_GRAPH_CONFIGURATION_STATE_FAULTY, + "Graph is in a faulty state: %!+g", graph); bt_graph_set_can_consume(graph, false); status = bt_graph_configure(graph); if (status) { @@ -702,6 +708,8 @@ enum bt_graph_status bt_graph_run(struct bt_graph *graph) BT_ASSERT_PRE(!graph->canceled, "Graph is canceled: %!+g", graph); BT_ASSERT_PRE(graph->can_consume, "Cannot consume graph in its current state: %!+g", graph); + BT_ASSERT_PRE(graph->config_state != BT_GRAPH_CONFIGURATION_STATE_FAULTY, + "Graph is in a faulty state: %!+g", graph); bt_graph_set_can_consume(graph, false); status = bt_graph_configure(graph); if (status) { @@ -1249,7 +1257,7 @@ enum bt_graph_status add_component_with_init_method_data( BT_ASSERT_PRE(!graph->canceled, "Graph is canceled: %!+g", graph); BT_ASSERT_PRE( graph->config_state == BT_GRAPH_CONFIGURATION_STATE_CONFIGURING, - "Graph is already configured: %!+g", graph); + "Graph is not in the \"configuring\" state: %!+g", graph); BT_ASSERT_PRE(!component_name_exists(graph, name), "Duplicate component name: %!+g, name=\"%s\"", graph, name); BT_ASSERT_PRE(!params || bt_value_is_map(params), @@ -1335,6 +1343,10 @@ enum bt_graph_status add_component_with_init_method_data( } end: + if (graph_status != BT_GRAPH_STATUS_OK) { + graph->config_state = BT_GRAPH_CONFIGURATION_STATE_FAULTY; + } + bt_object_put_ref(component); bt_object_put_ref(new_params); (void) init_can_consume;