From kwin  Sat Jul 18 12:21:51 2009
From: Christoph Feck <christoph () maxiom ! de>
Date: Sat, 18 Jul 2009 12:21:51 +0000
To: kwin
Subject: [PATCH] Reentrancy problem in KWindowSystem
Message-Id: <200907181421.51978.christoph () maxiom ! de>
X-MARC-Message: https://marc.info/?l=kwin&m=124791974016062
MIME-Version: 1
Content-Type: multipart/mixed; boundary="--Boundary-00=_f5bYK2F/FB7NQgK"

--Boundary-00=_f5bYK2F/FB7NQgK
Content-Type: text/plain;
  charset="us-ascii"
Content-Transfer-Encoding: 7bit
Content-Disposition: inline

Hi (especially Lubos :),

Still trying to resolve bug 170519 I found a possible reason and fix.

The current implementation of update() uses some arrays to check which clients 
are added and removed. During this, it emit signals with its findings.

The problem is for example in ksystray app that connects to windowAdded() 
signal, and calls for example KWindowSystem::activate() during its connected 
handler. This in turn calls into update() of netwm.cpp, but this function is 
not prepared to be called again, as the p->clients list is still in use.

The attached patch tries to fix it, and I am looking for comments, as I am not 
really familiar with the stuff.

Thanks,
Christoph Feck (kdepepo)

References:

https://bugs.kde.org/show_bug.cgi?id=170519
https://bugs.kde.org/show_bug.cgi?id=196552
https://bugs.kde.org/show_bug.cgi?id=199242

--Boundary-00=_f5bYK2F/FB7NQgK
Content-Type: text/x-diff;
  charset="iso 8859-15";
  name="windowmanagement.diff"
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment;
	filename="windowmanagement.diff"

Index: windowmanagement/netwm.cpp
===================================================================
--- windowmanagement/netwm.cpp	(Revision 998676)
+++ windowmanagement/netwm.cpp	(Arbeitskopie)
@@ -2108,6 +2108,9 @@
     }
 
     if (dirty & ClientList) {
+        QList<Window> clientsToRemove;
+        QList<Window> clientsToAdd;
+
         bool read_ok = false;
 	if (XGetWindowProperty(p->display, p->root, net_client_list,
 			       0l, MAX_PROP_SIZE, False, XA_WINDOW, &type_ret,
@@ -2126,16 +2129,16 @@
 
 			while (old_index < old_count || new_index < new_count) {
 			    if (old_index == old_count) {
-				addClient(wins[new_index++]);
+				clientsToAdd.append(wins[new_index++]);
 			    } else if (new_index == new_count) {
-				removeClient(p->clients[old_index++]);
+				clientsToRemove.append(p->clients[old_index++]);
 			    } else {
 				if (p->clients[old_index] <
 				    wins[new_index]) {
-				    removeClient(p->clients[old_index++]);
+				    clientsToRemove.append(p->clients[old_index++]);
 				} else if (wins[new_index] <
 					   p->clients[old_index]) {
-				    addClient(wins[new_index++]);
+				    clientsToAdd.append(wins[new_index++]);
 				} else {
 				    new_index++;
 				    old_index++;
@@ -2152,7 +2155,7 @@
 
 		    unsigned long n;
 		    for (n = 0; n < nitems_ret; n++) {
-			addClient(wins[n]);
+			clientsToAdd.append(wins[n]);
 		    }
 		}
 
@@ -2166,7 +2169,7 @@
 	}
         if( !read_ok ) {
             for( unsigned int i = 0; i < p->clients_count; ++ i )
-	        removeClient(p->clients[i]);
+	        clientsToRemove.append(p->clients[i]);
             p->clients_count = 0;
             delete[] p->clients;
             p->clients = NULL;
@@ -2176,6 +2179,12 @@
 	fprintf(stderr, "NETRootInfo::update: client list updated (%ld clients)\n",
 		p->clients_count);
 #endif
+        for (int i = 0; i < clientsToRemove.size(); ++i) {
+            removeClient(clientsToRemove.at(i));
+        }
+        for (int i = 0; i < clientsToAdd.size(); ++i) {
+            addClient(clientsToAdd.at(i));
+        }
     }
 
     if (dirty & ClientListStacking) {

--Boundary-00=_f5bYK2F/FB7NQgK
Content-Type: text/plain; charset="us-ascii"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Content-Disposition: inline

_______________________________________________
kwin mailing list
kwin@kde.org
https://mail.kde.org/mailman/listinfo/kwin

--Boundary-00=_f5bYK2F/FB7NQgK--