From: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Date: Fri, 10 Mar 2017 19:19:43 +0000 (-0500)
Subject: Fix: only remove newly added components on connection error
X-Git-Tag: v2.0.0-pre1~445
X-Git-Url: http://git.efficios.com/?p=babeltrace.git;a=commitdiff_plain;h=3eeacbb9265a17d3d3270fb44ae45ab8924034c6

Fix: only remove newly added components on connection error

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
---

diff --git a/lib/component/graph.c b/lib/component/graph.c
index 9982f5d0..51f3f957 100644
--- a/lib/component/graph.c
+++ b/lib/component/graph.c
@@ -96,6 +96,8 @@ struct bt_connection *bt_graph_connect(struct bt_graph *graph,
 	enum bt_component_status component_status;
 	bool upstream_was_already_in_graph;
 	bool downstream_was_already_in_graph;
+	int components_to_remove = 0;
+	int i;
 
 	if (!graph || !upstream_port || !downstream_port) {
 		goto end;
@@ -166,28 +168,84 @@ struct bt_connection *bt_graph_connect(struct bt_graph *graph,
 	component_status = bt_component_new_connection(upstream_component,
 			upstream_port, connection);
 	if (component_status != BT_COMPONENT_STATUS_OK) {
-		goto error;
+		goto error_rollback;
 	}
 	component_status = bt_component_new_connection(downstream_component,
 			downstream_port, connection);
 	if (component_status != BT_COMPONENT_STATUS_OK) {
-		goto error;
+		goto error_rollback;
 	}
 end:
 	bt_put(upstream_graph);
 	bt_put(downstream_graph);
+	bt_put(upstream_component);
+	bt_put(downstream_component);
 	return connection;
-error:
-	if (components_added) {
+error_rollback:
+	/*
+	 * Remove newly-added components from the graph, being careful
+	 * not to remove a component that was already present in the graph
+	 * and is connected to other components.
+	 */
+	components_to_remove += upstream_was_already_in_graph ? 0 : 1;
+	components_to_remove += downstream_was_already_in_graph ? 0 : 1;
+
+	if (!downstream_was_already_in_graph) {
 		if (bt_component_get_class_type(downstream_component) ==
 				BT_COMPONENT_CLASS_TYPE_SINK) {
 			g_queue_pop_tail(graph->sinks_to_consume);
 		}
-		g_ptr_array_set_size(graph->connections,
-				graph->connections->len - 1);
+	}
+	/* Remove newly created connection. */
+	g_ptr_array_set_size(graph->connections,
+			graph->connections->len - 1);
+
+	/*
+	 * Remove newly added components.
+	 *
+	 * Note that this is a tricky situation. The graph, being the parent
+	 * of the components, does not hold a reference to them. Normally,
+	 * components are destroyed right away when the graph is released since
+	 * the graph, being their parent, bounds their lifetime
+	 * (see doc/ref-counting.md).
+	 *
+	 * In this particular case, we must take a number of steps:
+	 *   1) unset the components' parent to rollback the initial state of
+	 *      the components being connected.
+	 *      Note that the reference taken by the component on its graph is
+	 *      released by the set_parent call.
+	 *   2) set the pointer in the components array to NULL so that the
+	 *      destruction function called on the array's resize in invoked on
+	 *      NULL (no effect),
+	 *
+	 * NOTE: Point #1 assumes that *something* holds a reference to both
+	 *       components being connected. The fact that a reference is being
+	 *       held to a component means that it must hold a reference to its
+	 *       parent to prevent the parent from being destroyed (again, refer
+	 *       to doc/red-counting.md). This reference to a component is
+	 *       most likely being held *transitively* by the caller which holds
+	 *       a reference to both ports (a port has its component as a
+	 *       parent).
+	 *
+	 *       This assumes that a graph is not connecting components by
+	 *       itself while not holding a reference to the ports/components
+	 *       being connected (i.e. "cheating" by using internal APIs).
+	 */
+	for (i = 0; i < components_to_remove; i++) {
+		struct bt_component *component = g_ptr_array_index(
+				graph->components, graph->components->len - 1);
+
+		bt_component_set_graph(component, NULL);
+		g_ptr_array_index(graph->components,
+				graph->components->len - 1) = NULL;
 		g_ptr_array_set_size(graph->components,
-				graph->components->len - 2);
+				graph->components->len - 1);
 	}
+	/* NOTE: Resizing the ptr_arrays invokes the destruction of the elements. */
+	goto end;
+error:
+	BT_PUT(upstream_component);
+	BT_PUT(downstream_component);
 	goto end;
 }