Merge branch 'present' of github.com:CendioOssman/tigervnc

13 files changed, 173 insertions, 115 deletions

diff --git a/common/rfb/EncodeManager.cxx b/common/rfb/EncodeManager.cxx
index bc15e74a..c2658a70 100644
--- a/common/rfb/EncodeManager.cxx
+++ b/common/rfb/EncodeManager.cxx
@@ -297,7 +297,7 @@ void EncodeManager::writeLosslessRefresh(const Region& req, const PixelBuffer* p
            Region(), Point(), pb, renderedCursor);
 }
 
-bool EncodeManager::handleTimeout(Timer* t)
+void EncodeManager::handleTimeout(Timer* t)
 {
   if (t == &recentChangeTimer) {
     // Any lossy region that wasn't recently updated can
@@ -307,10 +307,8 @@ bool EncodeManager::handleTimeout(Timer* t)
 
     // Will there be more to do? (i.e. do we need another round)
     if (!lossyRegion.subtract(pendingRefreshRegion).is_empty())
-      return true;
+      t->repeat();
   }
-
-  return false;
 }
 
 void EncodeManager::doUpdate(bool allowLossy, const Region& changed_,
diff --git a/common/rfb/EncodeManager.h b/common/rfb/EncodeManager.h
index f2fd4ca4..33484db8 100644
--- a/common/rfb/EncodeManager.h
+++ b/common/rfb/EncodeManager.h
@@ -61,7 +61,7 @@ namespace rfb {
                               size_t maxUpdateSize);
 
   protected:
-    virtual bool handleTimeout(Timer* t);
+    virtual void handleTimeout(Timer* t);
 
     void doUpdate(bool allowLossy, const Region& changed,
                   const Region& copied, const Point& copy_delta,
diff --git a/common/rfb/SConnection.cxx b/common/rfb/SConnection.cxx
index 7a930af5..12ba0f1a 100644
--- a/common/rfb/SConnection.cxx
+++ b/common/rfb/SConnection.cxx
@@ -275,11 +275,11 @@ bool SConnection::processInitMsg()
   return reader_->readClientInit();
 }
 
-bool SConnection::handleAuthFailureTimeout(Timer* /*t*/)
+void SConnection::handleAuthFailureTimeout(Timer* /*t*/)
 {
   if (state_ != RFBSTATE_SECURITY_FAILURE) {
     close("SConnection::handleAuthFailureTimeout: invalid state");
-    return false;
+    return;
   }
 
   try {
@@ -292,12 +292,10 @@ bool SConnection::handleAuthFailureTimeout(Timer* /*t*/)
     os->flush();
   } catch (rdr::Exception& e) {
     close(e.str());
-    return false;
+    return;
   }
 
   close(authFailureMsg.c_str());
-
-  return false;
 }
 
 void SConnection::throwConnFailedException(const char* format, ...)
diff --git a/common/rfb/SConnection.h b/common/rfb/SConnection.h
index cc88cd1e..5bc61677 100644
--- a/common/rfb/SConnection.h
+++ b/common/rfb/SConnection.h
@@ -238,7 +238,7 @@ namespace rfb {
     bool processSecurityFailure();
     bool processInitMsg();
 
-    bool handleAuthFailureTimeout(Timer* t);
+    void handleAuthFailureTimeout(Timer* t);
 
     int defaultMajorVersion, defaultMinorVersion;
 
diff --git a/common/rfb/SDesktop.h b/common/rfb/SDesktop.h
index 9db08116..94e4b028 100644
--- a/common/rfb/SDesktop.h
+++ b/common/rfb/SDesktop.h
@@ -1,5 +1,5 @@
 /* Copyright (C) 2002-2005 RealVNC Ltd.  All Rights Reserved.
- * Copyright 2009-2019 Pierre Ossman for Cendio AB
+ * Copyright 2009-2024 Pierre Ossman for Cendio AB
  * 
  * This is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License as published by
@@ -49,20 +49,21 @@ namespace rfb {
 
   class SDesktop : public InputHandler {
   public:
+    // init() is called immediately when the VNCServer gets a reference
+    // to the SDesktop, so that a reverse reference can be set up.
+    virtual void init(rfb::VNCServer* vs) = 0;
+
     // start() is called by the server when the first client authenticates
     // successfully, and can be used to begin any expensive tasks which are not
     // needed when there are no clients.  A valid PixelBuffer must have been
     // set via the VNCServer's setPixelBuffer() method by the time this call
     // returns.
-
-    virtual void start(VNCServer* vs) = 0;
+    virtual void start() {}
 
     // stop() is called by the server when there are no longer any
     // authenticated clients, and therefore the desktop can cease any
-    // expensive tasks.  No further calls to the VNCServer passed to start()
-    // can be made once stop has returned.
-
-    virtual void stop() = 0;
+    // expensive tasks.
+    virtual void stop() {}
 
     // queryConnection() is called when a connection has been
     // successfully authenticated.  The sock and userName arguments
@@ -86,6 +87,10 @@ namespace rfb {
       return resultProhibited;
     }
 
+    // frameTick() is called whenever a frame update has been processed,
+    // signalling that a good time to render new data
+    virtual void frameTick(uint64_t msc) { (void)msc; }
+
     // InputHandler interface
     // pointerEvent(), keyEvent() and clientCutText() are called in response to
     // the relevant RFB protocol messages from clients.
@@ -136,14 +141,10 @@ namespace rfb {
       if (buffer) delete buffer;
     }
 
-    virtual void start(VNCServer* vs) {
+    virtual void init(VNCServer* vs) {
       server = vs;
       server->setPixelBuffer(buffer);
     }
-    virtual void stop() {
-      server->setPixelBuffer(0);
-      server = 0;
-    }
     virtual void queryConnection(network::Socket* sock,
                                  const char* /*userName*/) {
       server->approveConnection(sock, true, NULL);
diff --git a/common/rfb/Timer.cxx b/common/rfb/Timer.cxx
index 4ff15bc5..e9ae5227 100644
--- a/common/rfb/Timer.cxx
+++ b/common/rfb/Timer.cxx
@@ -1,5 +1,5 @@
 /* Copyright (C) 2002-2005 RealVNC Ltd.  All Rights Reserved.
- * Copyright 2016-2018 Pierre Ossman for Cendio AB
+ * Copyright 2016-2024 Pierre Ossman for Cendio AB
  * 
  * This is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License as published by
@@ -61,36 +61,20 @@ int Timer::checkTimeouts() {
   timeval start;
 
   if (pending.empty())
-    return 0;
+    return -1;
 
   gettimeofday(&start, 0);
   while (pending.front()->isBefore(start)) {
     Timer* timer;
-    timeval before;
 
     timer = pending.front();
     pending.pop_front();
 
-    gettimeofday(&before, 0);
-    if (timer->cb->handleTimeout(timer)) {
-      timeval now;
+    timer->lastDueTime = timer->dueTime;
+    timer->cb->handleTimeout(timer);
 
-      gettimeofday(&now, 0);
-
-      timer->dueTime = addMillis(timer->dueTime, timer->timeoutMs);
-      if (timer->isBefore(now)) {
-        // Time has jumped forwards, or we're not getting enough
-        // CPU time for the timers
-
-        timer->dueTime = addMillis(before, timer->timeoutMs);
-        if (timer->isBefore(now))
-          timer->dueTime = now;
-      }
-
-      insertTimer(timer);
-    } else if (pending.empty()) {
-      return 0;
-    }
+    if (pending.empty())
+      return -1;
   }
   return getNextTimeout();
 }
@@ -98,7 +82,12 @@ int Timer::checkTimeouts() {
 int Timer::getNextTimeout() {
   timeval now;
   gettimeofday(&now, 0);
-  int toWait = __rfbmax(1, pending.front()->getRemainingMs());
+
+  if (pending.empty())
+    return -1;
+
+  int toWait = pending.front()->getRemainingMs();
+
   if (toWait > pending.front()->timeoutMs) {
     if (toWait - pending.front()->timeoutMs < 1000) {
       vlog.info("gettimeofday is broken...");
@@ -107,8 +96,9 @@ int Timer::getNextTimeout() {
     // Time has jumped backwards!
     vlog.info("time has moved backwards!");
     pending.front()->dueTime = now;
-    toWait = 1;
+    toWait = 0;
   }
+
   return toWait;
 }
 
@@ -128,13 +118,36 @@ void Timer::start(int timeoutMs_) {
   gettimeofday(&now, 0);
   stop();
   timeoutMs = timeoutMs_;
-  // The rest of the code assumes non-zero timeout
-  if (timeoutMs <= 0)
-    timeoutMs = 1;
   dueTime = addMillis(now, timeoutMs);
   insertTimer(this);
 }
 
+void Timer::repeat(int timeoutMs_) {
+  timeval now;
+
+  gettimeofday(&now, 0);
+
+  if (isStarted()) {
+    vlog.error("Incorrectly repeating already running timer");
+    stop();
+  }
+
+  if (msBetween(&lastDueTime, &dueTime) != 0)
+    vlog.error("Timer incorrectly modified whilst repeating");
+
+  if (timeoutMs_ != -1)
+    timeoutMs = timeoutMs_;
+
+  dueTime = addMillis(lastDueTime, timeoutMs);
+  if (isBefore(now)) {
+    // Time has jumped forwards, or we're not getting enough
+    // CPU time for the timers
+    dueTime = now;
+  }
+
+  insertTimer(this);
+}
+
 void Timer::stop() {
   pending.remove(this);
 }
diff --git a/common/rfb/Timer.h b/common/rfb/Timer.h
index ddfce1b2..36ec46c5 100644
--- a/common/rfb/Timer.h
+++ b/common/rfb/Timer.h
@@ -1,5 +1,5 @@
 /* Copyright (C) 2002-2005 RealVNC Ltd.  All Rights Reserved.
- * Copyright 2018 Pierre Ossman for Cendio AB
+ * Copyright 2018-2024 Pierre Ossman for Cendio AB
  * 
  * This is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License as published by
@@ -27,93 +27,108 @@ namespace rfb {
 
   /* Timer
 
-     Cross-platform timeout handling.  The caller creates instances of Timer and passes a
-     Callback implementation to each.  The Callback will then be called with a pointer to
-     the Timer instance that timed-out when the timeout occurs.
+     Cross-platform timeout handling.  The caller creates instances of
+     Timer and passes a Callback implementation to each.  The Callback
+     will then be called with a pointer to the Timer instance that
+     timed-out when the timeout occurs.
 
-     The static methods of Timer are used by the main loop of the application both to
-     dispatch elapsed Timer callbacks and to determine how long to wait in select() for
-     the next timeout to occur.
+     The static methods of Timer are used by the main loop of the
+     application both to dispatch elapsed Timer callbacks and to
+     determine how long to wait in select() for the next timeout to
+     occur.
 
-     For classes that can be derived it's best to use MethodTimer which can call a specific
-     method on the class, thus avoiding conflicts when subclassing.
+     For classes that can be derived it's best to use MethodTimer which
+     can call a specific method on the class, thus avoiding conflicts
+     when subclassing.
   */
 
   struct Timer {
 
     struct Callback {
       // handleTimeout
-      //   Passed a pointer to the Timer that has timed out.  If the handler returns true
-      //   then the Timer is reset and left running, causing another timeout after the
-      //   appropriate interval.
+      //   Passed a pointer to the Timer that has timed out.  If the
+      //   handler returns true then the Timer is reset and left
+      //   running, causing another timeout after the appropriate
+      //   interval.
       //   If the handler returns false then the Timer is cancelled.
-      virtual bool handleTimeout(Timer* t) = 0;
+      virtual void handleTimeout(Timer* t) = 0;
 
       virtual ~Callback() {}
     };
 
     // checkTimeouts()
-    //   Dispatches any elapsed Timers, and returns the number of milliseconds until the
-    //   next Timer will timeout.
+    //   Dispatches any elapsed Timers, and returns the number of
+    //   milliseconds until the next Timer will timeout.
     static int checkTimeouts();
 
     // getNextTimeout()
-    //   Returns the number of milliseconds until the next timeout, without dispatching
-    //   any elapsed Timers.
+    //   Returns the number of milliseconds until the next timeout,
+    //   without dispatching any elapsed Timers.
     static int getNextTimeout();
 
     // Create a Timer with the specified callback handler
     Timer(Callback* cb_) {cb = cb_;}
     ~Timer() {stop();}
 
-    // startTimer
-    //   Starts the timer, causing a timeout after the specified number of milliseconds.
-    //   If the timer is already active then it will be implicitly cancelled and re-started.
+    // start()
+    //   Starts the timer, causing a timeout after the specified number
+    //   of milliseconds. If the timer is already active then it will
+    //   be implicitly cancelled and re-started.
     void start(int timeoutMs_);
 
-    // stopTimer
+    // repeat()
+    //   Restarts the timer in a way that repeats that last timeout.
+    //   This allows you to have a periodic timer without the risk of
+    //   accumulating drift caused by processing delays.
+    //   A new interval can be specified, otherwise the previous
+    //   interval is reused.
+    void repeat(int timeoutMs_=-1);
+
+    // stop()
     //   Cancels the timer.
     void stop();
 
-    // isStarted
+    // isStarted()
     //   Determines whether the timer is started.
     bool isStarted();
 
-    // getTimeoutMs
+    // getTimeoutMs()
     //   Determines the previously used timeout value, if any.
     //   Usually used with isStarted() to get the _current_ timeout.
     int getTimeoutMs();
 
-    // getRemainingMs
+    // getRemainingMs()
     //   Determines how many milliseconds are left before the Timer
     //   will timeout. Only valid for an active timer.
     int getRemainingMs();
 
-    // isBefore
-    //   Determine whether the Timer will timeout before the specified time.
+    // isBefore()
+    //   Determine whether the Timer will timeout before the specified
+    //   time.
     bool isBefore(timeval other);
 
   protected:
-    timeval dueTime;
+    timeval dueTime, lastDueTime;
     int timeoutMs;
     Callback* cb;
 
     static void insertTimer(Timer* t);
-    // The list of currently active Timers, ordered by time left until timeout.
+    // The list of currently active Timers, ordered by time left until
+    // timeout.
     static std::list<Timer*> pending;
   };
 
   template<class T> class MethodTimer
     : public Timer, public Timer::Callback {
   public:
-    MethodTimer(T* obj_, bool (T::*cb_)(Timer*))
+    MethodTimer(T* obj_, void (T::*cb_)(Timer*))
       : Timer(this), obj(obj_), cb(cb_) {}
 
-    virtual bool handleTimeout(Timer* t) { return (obj->*cb)(t); }
+    virtual void handleTimeout(Timer* t) { (obj->*cb)(t); }
 
   private:
     T* obj;
-    bool (T::*cb)(Timer*);
+    void (T::*cb)(Timer*);
   };
 
 };
diff --git a/common/rfb/VNCSConnectionST.cxx b/common/rfb/VNCSConnectionST.cxx
index f1194eb6..306bba1d 100644
--- a/common/rfb/VNCSConnectionST.cxx
+++ b/common/rfb/VNCSConnectionST.cxx
@@ -800,7 +800,7 @@ void VNCSConnectionST::supportsLEDState()
   writer()->writeLEDState();
 }
 
-bool VNCSConnectionST::handleTimeout(Timer* t)
+void VNCSConnectionST::handleTimeout(Timer* t)
 {
   try {
     if ((t == &congestionTimer) ||
@@ -812,8 +812,6 @@ bool VNCSConnectionST::handleTimeout(Timer* t)
 
   if (t == &idleTimer)
     close("Idle timeout");
-
-  return false;
 }
 
 bool VNCSConnectionST::isShiftPressed()
diff --git a/common/rfb/VNCSConnectionST.h b/common/rfb/VNCSConnectionST.h
index 2f117a75..3a9ec242 100644
--- a/common/rfb/VNCSConnectionST.h
+++ b/common/rfb/VNCSConnectionST.h
@@ -141,7 +141,7 @@ namespace rfb {
     virtual void supportsLEDState();
 
     // Timer callbacks
-    virtual bool handleTimeout(Timer* t);
+    virtual void handleTimeout(Timer* t);
 
     // Internal methods
 
diff --git a/common/rfb/VNCServer.h b/common/rfb/VNCServer.h
index cf14bd86..3ac9fb94 100644
--- a/common/rfb/VNCServer.h
+++ b/common/rfb/VNCServer.h
@@ -73,6 +73,9 @@ namespace rfb {
     virtual void blockUpdates() = 0;
     virtual void unblockUpdates() = 0;
 
+    virtual uint64_t getMsc() = 0;
+    virtual void queueMsc(uint64_t target) = 0;
+
     // setPixelBuffer() tells the server to use the given pixel buffer (and
     // optionally a modified screen layout).  If this differs in size from
     // the previous pixel buffer, this may result in protocol messages being
diff --git a/common/rfb/VNCServerST.cxx b/common/rfb/VNCServerST.cxx
index fb421068..b9579f12 100644
--- a/common/rfb/VNCServerST.cxx
+++ b/common/rfb/VNCServerST.cxx
@@ -1,5 +1,5 @@
 /* Copyright (C) 2002-2005 RealVNC Ltd.  All Rights Reserved.
- * Copyright 2009-2019 Pierre Ossman for Cendio AB
+ * Copyright 2009-2024 Pierre Ossman for Cendio AB
  * 
  * This is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License as published by
@@ -90,10 +90,12 @@ VNCServerST::VNCServerST(const char* name_, SDesktop* desktop_)
     renderedCursorInvalid(false),
     keyRemapper(&KeyRemapper::defInstance),
     idleTimer(this), disconnectTimer(this), connectTimer(this),
-    frameTimer(this)
+    msc(0), queuedMsc(0), frameTimer(this)
 {
   slog.debug("creating single-threaded server %s", name.c_str());
 
+  desktop_->init(this);
+
   // FIXME: Do we really want to kick off these right away?
   if (rfb::Server::maxIdleTime)
     idleTimer.start(secsToMillis(rfb::Server::maxIdleTime));
@@ -248,11 +250,22 @@ void VNCServerST::unblockUpdates()
 
   blockCounter--;
 
-  // Restart the frame clock if we have updates
-  if (blockCounter == 0) {
-    if (!comparer->is_empty())
-      startFrameClock();
-  }
+  // Restart the frame clock in case we have updates
+  if (blockCounter == 0)
+    startFrameClock();
+}
+
+uint64_t VNCServerST::getMsc()
+{
+  return msc;
+}
+
+void VNCServerST::queueMsc(uint64_t target)
+{
+  if (target > queuedMsc)
+    queuedMsc = target;
+
+  startFrameClock();
 }
 
 void VNCServerST::setPixelBuffer(PixelBuffer* pb_, const ScreenSet& layout)
@@ -623,22 +636,33 @@ SConnection* VNCServerST::getConnection(network::Socket* sock) {
   return 0;
 }
 
-bool VNCServerST::handleTimeout(Timer* t)
+void VNCServerST::handleTimeout(Timer* t)
 {
   if (t == &frameTimer) {
-    // We keep running until we go a full interval without any updates
-    if (comparer->is_empty())
-      return false;
+    int timeout;
 
-    writeUpdate();
+    // We keep running until we go a full interval without any updates,
+    // or there are no active clients anymore
+    if (comparer->is_empty() || !desktopStarted) {
+      // Unless something waits for us to advance the frame count
+      if (queuedMsc < msc)
+        return;
+    }
 
     // If this is the first iteration then we need to adjust the timeout
-    if (frameTimer.getTimeoutMs() != 1000/rfb::Server::frameRate) {
-      frameTimer.start(1000/rfb::Server::frameRate);
-      return false;
-    }
+    timeout = 1000/rfb::Server::frameRate;
 
-    return true;
+    // If there are no clients, then slow down the clock
+    if (!desktopStarted)
+      timeout = 1000;
+
+    frameTimer.repeat(timeout);
+
+    if (!comparer->is_empty() && desktopStarted)
+      writeUpdate();
+
+    msc++;
+    desktop->frameTick(msc);
   } else if (t == &idleTimer) {
     slog.info("MaxIdleTime reached, exiting");
     desktop->terminate();
@@ -649,8 +673,6 @@ bool VNCServerST::handleTimeout(Timer* t)
     slog.info("MaxConnectionTime reached, exiting");
     desktop->terminate();
   }
-
-  return false;
 }
 
 void VNCServerST::queryConnection(VNCSConnectionST* client,
@@ -714,7 +736,7 @@ void VNCServerST::startDesktop()
 {
   if (!desktopStarted) {
     slog.debug("starting desktop");
-    desktop->start(this);
+    desktop->start();
     if (!pb)
       throw Exception("SDesktop::start() did not set a valid PixelBuffer");
     desktopStarted = true;
@@ -722,6 +744,12 @@ void VNCServerST::startDesktop()
     // stopped, so flush those out
     if (!comparer->is_empty())
       writeUpdate();
+    // If the frame clock is running, then it will be running slowly,
+    // so give it a kick to run at normal speed right away
+    if (frameTimer.isStarted()) {
+      stopFrameClock();
+      startFrameClock();
+    }
   }
 }
 
@@ -731,7 +759,6 @@ void VNCServerST::stopDesktop()
     slog.debug("stopping desktop");
     desktopStarted = false;
     desktop->stop();
-    stopFrameClock();
   }
 }
 
@@ -759,9 +786,18 @@ void VNCServerST::startFrameClock()
     return;
   if (blockCounter > 0)
     return;
-  if (!desktopStarted)
+
+  // Anyone actually interested in frames?
+  if (comparer->is_empty() && (queuedMsc <= msc))
     return;
 
+  // Run the frame clock very slowly if there are no clients to actually
+  // send updates to
+  if (!desktopStarted) {
+    frameTimer.start(1000);
+    return;
+  }
+
   // The first iteration will be just half a frame as we get a very
   // unstable update rate if we happen to be perfectly in sync with
   // the application's update rate
diff --git a/common/rfb/VNCServerST.h b/common/rfb/VNCServerST.h
index d303831e..90c8d753 100644
--- a/common/rfb/VNCServerST.h
+++ b/common/rfb/VNCServerST.h
@@ -79,6 +79,8 @@ namespace rfb {
 
     virtual void blockUpdates();
     virtual void unblockUpdates();
+    virtual uint64_t getMsc();
+    virtual void queueMsc(uint64_t target);
     virtual void setPixelBuffer(PixelBuffer* pb, const ScreenSet& layout);
     virtual void setPixelBuffer(PixelBuffer* pb);
     virtual void setScreenLayout(const ScreenSet& layout);
@@ -153,7 +155,7 @@ namespace rfb {
   protected:
 
     // Timer callbacks
-    virtual bool handleTimeout(Timer* t);
+    virtual void handleTimeout(Timer* t);
 
     // - Internal methods
 
@@ -204,6 +206,7 @@ namespace rfb {
     Timer disconnectTimer;
     Timer connectTimer;
 
+    uint64_t msc, queuedMsc;
     Timer frameTimer;
   };
 
diff --git a/common/rfb/util.h b/common/rfb/util.h
index cafea209..b47ac4c9 100644
--- a/common/rfb/util.h
+++ b/common/rfb/util.h
@@ -73,13 +73,6 @@ namespace rfb {
 
   // HELPER functions for timeout handling
 
-  // soonestTimeout() is a function to help work out the soonest of several
-  //   timeouts.
-  inline void soonestTimeout(int* timeout, int newTimeout) {
-    if (newTimeout && (!*timeout || newTimeout < *timeout))
-      *timeout = newTimeout;
-  }
-
   // secsToMillis() turns seconds into milliseconds, capping the value so it
   //   can't wrap round and become -ve
   inline int secsToMillis(int secs) {