Fix a race condition where we might get updates thrown at us right after a

framebuffer switch, but before we've been given the pointer to the new
framebuffer.


git-svn-id: svn://svn.code.sf.net/p/tigervnc/code/trunk@4839 3789f03b-4d11-0410-bbf8-ca57d06f2519

common/rfb/VNCServer.h

@@ -30,6 +30,13 @@ namespace rfb {
 
   class VNCServer : public UpdateTracker {
   public:
+    // blockUpdates()/unblockUpdates() tells the server that the pixel buffer
+    // is currently in flux and may not be accessed. The attributes of the
+    // pixel buffer may still be accessed, but not the frame buffer itself.
+    // Note that access must be unblocked the exact same number of times it
+    // was blocked.
+    virtual void blockUpdates() = 0;
+    virtual void unblockUpdates() = 0;
 
     // setPixelBuffer() tells the server to use the given pixel buffer (and
     // optionally a modified screen layout).  If this differs in size from

common/rfb/VNCServerST.cxx

@@ -47,6 +47,7 @@
 // otherwise blacklisted connections might be "forgotten".
 
 
+#include <assert.h>
 #include <stdlib.h>
 
 #include <rfb/ServerCore.h>
@@ -77,7 +78,8 @@ rfb::BoolParameter alwaysSetDeferUpdateTimer("AlwaysSetDeferUpdateTimer",
 // -=- Constructors/Destructor
 
 VNCServerST::VNCServerST(const char* name_, SDesktop* desktop_)
-  : blHosts(&blacklist), desktop(desktop_), desktopStarted(false), pb(0),
+  : blHosts(&blacklist), desktop(desktop_), desktopStarted(false),
+    blockCounter(0), pb(0),
     name(strDup(name_)), pointerClient(0), comparer(0),
     renderedCursorInvalid(false),
     queryConnectionHandler(0), keyRemapper(&KeyRemapper::defInstance),
@@ -259,6 +261,22 @@ int VNCServerST::checkTimeouts()
 
 // VNCServer methods
 
+void VNCServerST::blockUpdates()
+{
+  blockCounter++;
+}
+
+void VNCServerST::unblockUpdates()
+{
+  assert(blockCounter > 0);
+
+  blockCounter--;
+
+  // Flush out any updates we might have blocked
+  if (blockCounter == 0)
+    tryUpdate();
+}
+
 void VNCServerST::setPixelBuffer(PixelBuffer* pb_, const ScreenSet& layout)
 {
   pb = pb_;
@@ -545,6 +563,9 @@ void VNCServerST::tryUpdate()
 {
   std::list<VNCSConnectionST*>::iterator ci, ci_next;
 
+  if (blockCounter > 0)
+    return;
+
   if (!checkDefer())
     return;
 
@@ -571,6 +592,10 @@ bool VNCServerST::checkUpdate()
   if (ui.is_empty() && !(renderCursor && renderedCursorInvalid))
     return true;
 
+  // Block clients as the frame buffer cannot be safely accessed
+  if (blockCounter > 0)
+    return false;
+
   // Block client from updating if we are currently deferring updates
   if (!checkDefer())
     return false;

common/rfb/VNCServerST.h

@@ -82,6 +82,8 @@ namespace rfb {
 
     // Methods overridden from VNCServer
 
+    virtual void blockUpdates();
+    virtual void unblockUpdates();
     virtual void setPixelBuffer(PixelBuffer* pb, const ScreenSet& layout);
     virtual void setPixelBuffer(PixelBuffer* pb);
     virtual void setScreenLayout(const ScreenSet& layout);
@@ -209,6 +211,7 @@ namespace rfb {
 
     SDesktop* desktop;
     bool desktopStarted;
+    int blockCounter;
     PixelBuffer* pb;
     ScreenSet screenLayout;
 

unix/xserver/hw/vnc/XserverDesktop.cc

@@ -188,6 +188,16 @@ void XserverDesktop::serverReset(ScreenPtr pScreen_)
   cmap = (ColormapPtr) retval;
 }
 
+void XserverDesktop::blockUpdates()
+{
+  server->blockUpdates();
+}
+
+void XserverDesktop::unblockUpdates()
+{
+  server->unblockUpdates();
+}
+
 void XserverDesktop::setFramebuffer(int w, int h, void* fbptr, int stride)
 {
   width_ = w;

unix/xserver/hw/vnc/XserverDesktop.h

@@ -61,6 +61,8 @@ public:
 
   // methods called from X server code
   void serverReset(ScreenPtr pScreen);
+  void blockUpdates();
+  void unblockUpdates();
   void setFramebuffer(int w, int h, void* fbptr, int stride);
   void setColormap(ColormapPtr cmap);
   void setColourMapEntries(ColormapPtr pColormap, int ndef, xColorItem* pdef);

unix/xserver/hw/vnc/vncHooks.cc

@@ -605,6 +605,11 @@ static Bool vncHooksRandRSetConfig(ScreenPtr pScreen, Rotation rotation,
   RegionRec reg;
   BoxRec box;
 
+  // We need to prevent the RFB core from accessing the framebuffer
+  // for a while as there might be updates thrown our way inside
+  // rrSetConfig (i.e. before we have a pointer to the new framebuffer).
+  vncHooksScreen->desktop->blockUpdates();
+
   rp->rrSetConfig = vncHooksScreen->RandRSetConfig;
   ret = (*rp->rrSetConfig)(pScreen, rotation, rate, pSize);
   rp->rrSetConfig = vncHooksRandRSetConfig;
@@ -617,6 +622,8 @@ static Bool vncHooksRandRSetConfig(ScreenPtr pScreen, Rotation rotation,
                                           vncFbptr[pScreen->myNum],
                                           vncFbstride[pScreen->myNum]);
 
+  vncHooksScreen->desktop->unblockUpdates();
+
   // Mark entire screen as changed
   box.x1 = 0;
   box.y1 = 0;