diff options
author | Christopher Pitstick <cmp@pitstick.net> | 2021-05-17 09:24:03 +0300 |
---|---|---|
committer | Christopher Pitstick <cmp@pitstick.net> | 2021-05-17 09:34:26 +0300 |
commit | b90dd28c2995d62a5c6df73c4e1bbd5f50c7beb5 (patch) | |
tree | ccf4425835ef809a87f35977330c9dabdeebe9ee | |
parent | 547eaf38711357de34501b760c0372d2a75de749 (diff) |
Avoid potential infinite loop for rdpUpdate.
When frame acknowledgement is enabled, such as with the prototype of
egfx, the driver is at risk of going into an infinite loop if for some
reason an ack is missed.
While this line could fix it from xrdp: `mm->mod->mod_frame_ack(mm->mod, 0, INT_MAX);`,
the driver needs to be stable by itself.
The rationale here is that this is akin to a network socket timeout.
After a certain wait, it gives up. Same here.
Also, frame acknowledgement is a quality of service feature, and is not
strictly necessary for the functionality of remote desktop. In the worst
case, bandwidth calcuations between client and server are temporarily
incorrect.
200 was selected after some empirical testing, it could be tweaked.
-rw-r--r-- | module/rdpClientCon.c | 21 | ||||
-rw-r--r-- | module/rdpClientCon.h | 1 |
2 files changed, 16 insertions, 6 deletions
diff --git a/module/rdpClientCon.c b/module/rdpClientCon.c index 3d7e453..4357aea 100644 --- a/module/rdpClientCon.c +++ b/module/rdpClientCon.c @@ -33,6 +33,7 @@ Client connection to xrdp #include <sys/types.h> #include <sys/ipc.h> #include <sys/shm.h> +#include <limits.h> /* this should be before all X11 .h files */ #include <xorg-server.h> @@ -212,6 +213,7 @@ rdpClientConGotConnection(ScreenPtr pScreen, rdpPtr dev) LLOGLN(0, ("rdpClientConGotConnection:")); clientCon = g_new0(rdpClientCon, 1); clientCon->shmemstatus = SHM_UNINITIALIZED; + clientCon->updateRetries = 0; clientCon->dev = dev; dev->last_event_time_ms = GetTimeInMillis(); dev->do_dirty_ons = 1; @@ -2069,7 +2071,7 @@ rdpClientConAddOsBitmap(rdpPtr dev, rdpClientCon *clientCon, return -1; } - oldest = 0x7fffffff; + oldest = INT_MAX; oldest_index = -1; rv = -1; index = 0; @@ -2137,7 +2139,7 @@ rdpClientConAddOsBitmap(rdpPtr dev, rdpClientCon *clientCon, "clientCon->osBitmapNumUsed %d", clientCon->osBitmapNumUsed)); /* find oldest */ - oldest = 0x7fffffff; + oldest = INT_MAX; oldest_index = -1; index = 0; while (index < clientCon->maxOsBitmaps) @@ -2479,10 +2481,8 @@ rdpDeferredUpdateCallback(OsTimerPtr timer, CARD32 now, pointer arg) rdpScheduleDeferredUpdate(clientCon); return 0; } - else - { - LLOGLN(10, ("rdpDeferredUpdateCallback: sending")); - } + LLOGLN(10, ("rdpDeferredUpdateCallback: sending")); + clientCon->updateRetries = 0; rdpClientConGetScreenImageRect(clientCon->dev, clientCon, &id); LLOGLN(10, ("rdpDeferredUpdateCallback: rdp_width %d rdp_height %d " "rdp_Bpp %d screen width %d screen height %d", @@ -2578,6 +2578,7 @@ rdpDeferredUpdateCallback(OsTimerPtr timer, CARD32 now, pointer arg) /******************************************************************************/ #define MIN_MS_BETWEEN_FRAMES 40 #define MIN_MS_TO_WAIT_FOR_MORE_UPDATES 4 +#define UPDATE_RETRY_TIMEOUT 200 // After this number of retries, give up and perform the capture anyway. This prevents an infinite loop. static void rdpScheduleDeferredUpdate(rdpClientCon *clientCon) { @@ -2585,6 +2586,13 @@ rdpScheduleDeferredUpdate(rdpClientCon *clientCon) uint32_t msToWait; uint32_t minNextUpdateTime; + if (clientCon->updateRetries > UPDATE_RETRY_TIMEOUT) { + LLOGLN(10, ("rdpScheduleDeferredUpdate: clientCon->updateRetries is %d" + " and has exceeded the timeout of %d retries." + " Overriding rect_id_ack to INT_MAX.", clientCon->updateRetries, UPDATE_RETRY_TIMEOUT)); + clientCon->rect_id_ack = INT_MAX; + } + curTime = (uint32_t) GetTimeInMillis(); /* use two separate delays in order to limit the update rate and wait a bit for more changes before sending an update. Always waiting the longer @@ -2604,6 +2612,7 @@ rdpScheduleDeferredUpdate(rdpClientCon *clientCon) rdpDeferredUpdateCallback, clientCon); clientCon->updateScheduled = TRUE; + ++clientCon->updateRetries; } /******************************************************************************/ diff --git a/module/rdpClientCon.h b/module/rdpClientCon.h index 874870f..42b39c6 100644 --- a/module/rdpClientCon.h +++ b/module/rdpClientCon.h @@ -116,6 +116,7 @@ struct _rdpClientCon OsTimerPtr updateTimer; CARD32 lastUpdateTime; /* millisecond timestamp */ int updateScheduled; /* boolean */ + int updateRetries; RegionPtr dirtyRegion; |