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);