Blob Blame History Raw
rfbClientIterator is swarming with bugs:
* Whoever added rfbClientListMutex didn't know what he was doing.
* Reference counting is broken
* The iterator normally skips closed entries (those with sock<0).  But
  rfbClientConnectionGone() neglects to reset cl->sock.
* Closed entries are *not* skipped when LIBVNCSERVER_HAVE_LIBPTHREAD
  is undefined.

diff -rup a/LibVNCServer-0.8.2/libvncserver/rfbserver.c b/LibVNCServer-0.8.2/libvncserver/rfbserver.c
--- a/LibVNCServer-0.8.2/libvncserver/rfbserver.c	2007-07-31 16:00:10.000000000 +0200
+++ b/LibVNCServer-0.8.2/libvncserver/rfbserver.c	2007-07-31 15:14:47.000000000 +0200
@@ -148,40 +148,42 @@ rfbGetClientIteratorWithClosed(rfbScreen
 rfbClientPtr
 rfbClientIteratorHead(rfbClientIteratorPtr i)
 {
+  rfbClientPtr cl;
+  LOCK(rfbClientListMutex);
 #ifdef LIBVNCSERVER_HAVE_LIBPTHREAD
   if(i->next != 0) {
     rfbDecrClientRef(i->next);
-    rfbIncrClientRef(i->screen->clientHead);
   }
+  rfbIncrClientRef(i->screen->clientHead);
 #endif
-  LOCK(rfbClientListMutex);
-  i->next = i->screen->clientHead;
+  cl = i->next = i->screen->clientHead;
   UNLOCK(rfbClientListMutex);
-  return i->next;
+  return cl;
 }
 
 rfbClientPtr
 rfbClientIteratorNext(rfbClientIteratorPtr i)
 {
+  rfbClientPtr cl;
+  LOCK(rfbClientListMutex);
   if(i->next == 0) {
-    LOCK(rfbClientListMutex);
     i->next = i->screen->clientHead;
-    UNLOCK(rfbClientListMutex);
   } else {
-    IF_PTHREADS(rfbClientPtr cl = i->next);
+    IF_PTHREADS(cl = i->next);
     i->next = i->next->next;
     IF_PTHREADS(rfbDecrClientRef(cl));
   }
 
+  if(!i->closedToo)
+    while(i->next && i->next->sock<0)
+      i->next = i->next->next;
 #ifdef LIBVNCSERVER_HAVE_LIBPTHREAD
-    if(!i->closedToo)
-      while(i->next && i->next->sock<0)
-        i->next = i->next->next;
-    if(i->next)
-      rfbIncrClientRef(i->next);
+  if(i->next)
+    rfbIncrClientRef(i->next);
 #endif
-
-    return i->next;
+  cl = i->next;
+  UNLOCK(rfbClientListMutex);
+  return cl;
 }
 
 void
@@ -474,8 +476,11 @@ rfbClientConnectionGone(rfbClientPtr cl)
     if (cl->next)
         cl->next->prev = cl->prev;
 
-    if(cl->sock>0)
+    if(cl->sock>=0) {
+	FD_CLR(cl->sock,&(cl->screen->allFds));
 	close(cl->sock);
+	cl->sock = -1;
+    }
 
     if (cl->scaledScreen!=NULL)
         cl->scaledScreen->scaledScreenRefCount--;
@@ -501,9 +506,6 @@ rfbClientConnectionGone(rfbClientPtr cl)
 
     UNLOCK(rfbClientListMutex);
 
-    if(cl->sock>=0)
-       FD_CLR(cl->sock,&(cl->screen->allFds));
-
     cl->clientGoneHook(cl);
 
     rfbLog("Client %s gone\n",cl->host);