From eb8ad348b28e243cba1972e802ca8ee636472fc9 Mon Sep 17 00:00:00 2001
From: Michael Stapelberg <michael@stapelberg.de>
Date: Wed, 11 May 2011 20:22:47 +0200
Subject: [PATCH] =?utf8?q?Bugfix:=20Don=E2=80=99t=20run=20into=20an=20endl?=
 =?utf8?q?ess=20loop=20when=20killing=20con=20with=20children=20(Thanks=20?=
 =?utf8?q?mseed)?=
MIME-Version: 1.0
Content-Type: text/plain; charset=utf8
Content-Transfer-Encoding: 8bit

When a tabbed container had more than one child and at least the first one
supported WM_DELETE, i3 entered an endless loop when killing that tabbed
container. This was due to tree_close only sending WM_DELETE without actually
removing the child, while the loop in tree_close assumed that with every call
of tree_close one child would be removed.
---
 include/tree.h |  6 ++++--
 src/tree.c     | 28 ++++++++++++++++++++--------
 2 files changed, 24 insertions(+), 10 deletions(-)

diff --git a/include/tree.h b/include/tree.h
index 40d9a541..8ffeca0d 100644
--- a/include/tree.h
+++ b/include/tree.h
@@ -66,10 +66,12 @@ void tree_close_con();
 void tree_next(char way, orientation_t orientation);
 
 /**
- * Closes the given container including all children
+ * Closes the given container including all children.
+ * Returns true if the container was killed or false if just WM_DELETE was sent
+ * and the window is expected to kill itself.
  *
  */
-void tree_close(Con *con, bool kill_window, bool dont_kill_parent);
+bool tree_close(Con *con, bool kill_window, bool dont_kill_parent);
 
 /**
  * Loads tree from ~/.i3/_restart.json (used for in-place restarts).
diff --git a/src/tree.c b/src/tree.c
index b68af36b..f5024a55 100644
--- a/src/tree.c
+++ b/src/tree.c
@@ -94,10 +94,12 @@ static bool _is_con_mapped(Con *con) {
 }
 
 /*
- * Closes the given container including all children
+ * Closes the given container including all children.
+ * Returns true if the container was killed or false if just WM_DELETE was sent
+ * and the window is expected to kill itself.
  *
  */
-void tree_close(Con *con, bool kill_window, bool dont_kill_parent) {
+bool tree_close(Con *con, bool kill_window, bool dont_kill_parent) {
     bool was_mapped = con->mapped;
     Con *parent = con->parent;
 
@@ -113,19 +115,27 @@ void tree_close(Con *con, bool kill_window, bool dont_kill_parent) {
     DLOG("next = %p, focused = %p\n", next, focused);
 
     DLOG("closing %p, kill_window = %d\n", con, kill_window);
-    Con *child;
+    Con *child, *nextchild;
+    bool abort_kill = false;
     /* We cannot use TAILQ_FOREACH because the children get deleted
      * in their parent’s nodes_head */
-    while (!TAILQ_EMPTY(&(con->nodes_head))) {
-        child = TAILQ_FIRST(&(con->nodes_head));
+    for (child = TAILQ_FIRST(&(con->nodes_head)); child; ) {
+        nextchild = TAILQ_NEXT(child, nodes);
         DLOG("killing child=%p\n", child);
-        tree_close(child, kill_window, true);
+        if (!tree_close(child, kill_window, true))
+            abort_kill = true;
+        child = nextchild;
+    }
+
+    if (abort_kill) {
+        DLOG("One of the children could not be killed immediately (WM_DELETE sent), aborting.\n");
+        return false;
     }
 
     if (con->window != NULL) {
         if (kill_window) {
             x_window_kill(con->window->id);
-            return;
+            return false;
         } else {
             /* un-parent the window */
             xcb_reparent_window(conn, con->window->id, root, 0, 0);
@@ -175,7 +185,7 @@ void tree_close(Con *con, bool kill_window, bool dont_kill_parent) {
      * when closing the parent, so we can exit now. */
     if (!next) {
         DLOG("No next container, i will just exit now\n");
-        return;
+        return true;
     }
 
     if (was_mapped || con == focused) {
@@ -199,6 +209,8 @@ void tree_close(Con *con, bool kill_window, bool dont_kill_parent) {
     /* check if the parent container is empty now and close it */
     if (!dont_kill_parent)
         CALL(parent, on_remove_child);
+
+    return true;
 }
 
 /*
-- 
2.39.5