From 61cd498fb21b5b2a3f63f336b1f1ed297f451c22 Mon Sep 17 00:00:00 2001 From: dscho Date: Sat, 17 Mar 2007 00:13:12 +0000 Subject: [PATCH] Fix a locking problem in libvncserver There seems to be a locking problem in libvncserver, with respect to how condition variables are used. On certain machines in our lab, when using a vncviewer to view a display that has a very high rate of updates, we will occasionally see the VNC server process crash. In one stack trace that was obtained, an assertion had tripped in glibc's pthread_cond_wait, which was called from clientOutput. Inspection of clientOutput suggests that WAIT is being called incorrectly. The mutex that protects a condition variable should always be locked when calling wait, and on return from the wait will still be locked. The attached patch fixes the locking around this condition variable, and one other that I found by grepping the source for similar occurrences. Signed-off-by: Charles Coffing --- AUTHORS | 2 +- ChangeLog | 3 +++ libvncserver/main.c | 3 +-- libvncserver/rfbserver.c | 2 +- 4 files changed, 6 insertions(+), 4 deletions(-) diff --git a/AUTHORS b/AUTHORS index d5d1a87..ab799ef 100644 --- a/AUTHORS +++ b/AUTHORS @@ -29,7 +29,7 @@ Oliver Mihatsch, Greg Sternberg, Werner Hofer, Giampiero Giancipoli, Glenn Mabutt, Paul Kreiner, Erik Kunze, Mike Frysinger, Martin Waitz, Mark McLoughlin, Paul Fox, Juan Jose Costello, Andre Leiadella, Alberto Lusiani, Malvina Mazin, Dave Stuart, Rohit Kumar, Donald Dugger, -Steven Carr, and Uwe Völker. +Steven Carr, Uwe Völker, and Charles Coffing. Probably I forgot quite a few people sending a patch here and there, which really made a difference. Without those, some obscure bugs still would diff --git a/ChangeLog b/ChangeLog index d654589..b2f7a1d 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,6 @@ +2007-03-17 Charles Coffing + * libvncserver: fix a locking issue + 2007-02-01 Johannes E. Schindelin * libvncclient: add updateRect member to rfbClient, to allow requesting smaller updates than whole-screen. diff --git a/libvncserver/main.c b/libvncserver/main.c index 3af90ca..52bd4e7 100644 --- a/libvncserver/main.c +++ b/libvncserver/main.c @@ -455,12 +455,11 @@ clientOutput(void *data) haveUpdate = sraRgnAnd(updateRegion,cl->requestedRegion); sraRgnDestroy(updateRegion); } - UNLOCK(cl->updateMutex); if (!haveUpdate) { WAIT(cl->updateCond, cl->updateMutex); - UNLOCK(cl->updateMutex); /* we really needn't lock now. */ } + UNLOCK(cl->updateMutex); } /* OK, now, to save bandwidth, wait a little while for more diff --git a/libvncserver/rfbserver.c b/libvncserver/rfbserver.c index 0c7f584..434d59d 100644 --- a/libvncserver/rfbserver.c +++ b/libvncserver/rfbserver.c @@ -500,9 +500,9 @@ rfbClientConnectionGone(rfbClientPtr cl) do { LOCK(cl->refCountMutex); i=cl->refCount; - UNLOCK(cl->refCountMutex); if(i>0) WAIT(cl->deleteCond,cl->refCountMutex); + UNLOCK(cl->refCountMutex); } while(i>0); } #endif