Dispatch events while waiting for server data

Blocking the event loop is very bad

include/rfb/rfbclient.h

@@ -604,6 +604,8 @@ extern rfbBool SendClientCutText(rfbClient* client,char *str, int len);
  */
 extern rfbBool HandleRFBServerMessage(rfbClient* client);
 
+extern void ReadToBuffer(rfbClient* client);
+
 /**
  * Sends a text chat message to the server.
  * @param client The client through which to send the message

include/vnc.h

@@ -47,12 +47,15 @@ struct vnc_client {
 
 	void* userdata;
 	struct pixman_region16 damage;
+
+	bool handler_lock;
 };
 
 struct vnc_client* vnc_client_create(void);
 void vnc_client_destroy(struct vnc_client* self);
 
 int vnc_client_connect(struct vnc_client* self, const char* address, int port);
+int vnc_client_init(struct vnc_client* self);
 
 int vnc_client_set_pixel_format(struct vnc_client* self, uint32_t format);
 

src/main.c

@@ -670,6 +670,14 @@ failure:
 	return -1;
 }
 
+void run_main_loop_once(void)
+{
+	struct aml* aml = aml_get_default();
+	wl_display_flush(wl_display);
+	aml_poll(aml, -1);
+	aml_dispatch(aml);
+}
+
 static int usage(int r)
 {
 	fprintf(r ? stderr : stdout, "\
@@ -842,16 +850,18 @@ int main(int argc, char* argv[])
 	if (init_vnc_client_handler(vnc) < 0)
 		goto vnc_setup_failure;
 
+	if (vnc_client_init(vnc) < 0) {
+		fprintf(stderr, "Failed to connect to server\n");
+		goto vnc_setup_failure;
+	}
+
 	pointers->userdata = vnc;
 	keyboards->userdata = vnc;
 
 	wl_display_dispatch(wl_display);
 
-	while (do_run) {
+	while (do_run)
-		wl_display_flush(wl_display);
+		run_main_loop_once();
-		aml_poll(aml, -1);
-		aml_dispatch(aml);
-	}
 
 	rc = 0;
 	if (window)

src/sockets.c

@@ -1,4 +1,5 @@
 /*
+ *  Copyright (C) 2022 Andri Yngvasin <andri@yngvason.is>
  *  Copyright (C) 2011-2012 Christian Beier <dontmind@freeshell.org>
  *  Copyright (C) 1999 AT&T Laboratories Cambridge.  All Rights Reserved.
  *
@@ -34,206 +35,72 @@
 #include <errno.h>
 #include <fcntl.h>
 #include <assert.h>
-#include <rfb/rfbclient.h>
+#include <sys/param.h>
+#include "rfb/rfbclient.h"
 #include "sockets.h"
 #include "tls.h"
 #include "sasl.h"
 
+void run_main_loop_once(void);
+
 void PrintInHex(char *buf, int len);
 
 rfbBool errorMessageOnReadFailure = TRUE;
 
-/*
+void ReadToBuffer(rfbClient* client) {
- * ReadFromRFBServer is called whenever we want to read some data from the RFB
+	if (client->buffered == RFB_BUF_SIZE)
- * server.  It is non-trivial for two reasons:
+		return;
- *
- * 1. For efficiency it performs some intelligent buffering, avoiding invoking
- *    the read() system call too often.  For small chunks of data, it simply
- *    copies the data out of an internal buffer.  For large amounts of data it
- *    reads directly into the buffer provided by the caller.
- *
- * 2. Whenever read() would block, it invokes the Xt event dispatching
- *    mechanism to process X events.  In fact, this is the only place these
- *    events are processed, as there is no XtAppMainLoop in the program.
- */
 
-rfbBool
+	ssize_t size;
-ReadFromRFBServer(rfbClient* client, char *out, unsigned int n)
+
+#if defined(LIBVNCSERVER_HAVE_GNUTLS) || defined(LIBVNCSERVER_HAVE_LIBSSL)
+	if (client->tlsSession) {
+		size = ReadFromTLS(client, client->buf + client->buffered,
+				RFB_BUF_SIZE - client->buffered);
+	} else
+#endif
+#ifdef LIBVNCSERVER_HAVE_SASL
+	if (client->saslconn) {
+		size = ReadFromSASL(client, client->buf + client->buffered,
+				RFB_BUF_SIZE - client->buffered);
+	} else
+#endif
+	{
+		size = recv(client->sock, client->buf + client->buffered,
+				RFB_BUF_SIZE - client->buffered, MSG_DONTWAIT);
+	}
+
+	if (size > 0)
+		client->buffered += size;
+}
+
+rfbBool ReadFromRFBServer(rfbClient* client, char *out, unsigned int n)
 {
-  const int USECS_WAIT_PER_RETRY = 100000;
+	if (!out)
-  int retries = 0;
-#undef DEBUG_READ_EXACT
-#ifdef DEBUG_READ_EXACT
-	char* oout=out;
-	unsigned int nn=n;
-	rfbClientLog("ReadFromRFBServer %d bytes\n",n);
-#endif
-
-  /* Handle attempts to write to NULL out buffer that might occur
-     when an outside malloc() fails. For instance, memcpy() to NULL
-     results in undefined behaviour and probably memory corruption.*/
-  if(!out)
 		return FALSE;
 
-  if (client->serverPort==-1) {
+	while (n != 0) {
-    /* vncrec playing */
+		while (n != 0 && client->buffered == 0) {
-    rfbVNCRec* rec = client->vncRec;
+			run_main_loop_once();
-    struct timeval tv;
+			ReadToBuffer(client);
-
-    if (rec->readTimestamp) {
-      rec->readTimestamp = FALSE;
-      if (!fread(&tv,sizeof(struct timeval),1,rec->file))
-        return FALSE;
-
-      tv.tv_sec = rfbClientSwap32IfLE (tv.tv_sec);
-      tv.tv_usec = rfbClientSwap32IfLE (tv.tv_usec);
-
-      if (rec->tv.tv_sec!=0 && !rec->doNotSleep) {
-        struct timeval diff;
-        diff.tv_sec = tv.tv_sec - rec->tv.tv_sec;
-        diff.tv_usec = tv.tv_usec - rec->tv.tv_usec;
-        if(diff.tv_usec<0) {
-	  diff.tv_sec--;
-	  diff.tv_usec+=1000000;
-        }
-        sleep (diff.tv_sec);
-        usleep (diff.tv_usec);
 		}
 
-      rec->tv=tv;
+		unsigned int size = MIN(client->buffered, n);
-    }
+		memcpy(out, client->buf, size);
 
-    return (fread(out,1,n,rec->file) != n ? FALSE : TRUE);
+		client->buffered -= size;
-  }
+		memmove(client->buf, client->buf + size, client->buffered);
 
-  if (n <= client->buffered) {
+		out += size;
-    memcpy(out, client->bufoutptr, n);
+		n -= size;
-    client->bufoutptr += n;
-    client->buffered -= n;
-#ifdef DEBUG_READ_EXACT
-    goto hexdump;
-#endif
-    return TRUE;
 	}
 
-  memcpy(out, client->bufoutptr, client->buffered);
-
-  out += client->buffered;
-  n -= client->buffered;
-
-  client->bufoutptr = client->buf;
-  client->buffered = 0;
-
-  if (n <= RFB_BUF_SIZE) {
-
-    while (client->buffered < n) {
-      int i;
-      if (client->tlsSession)
-        i = ReadFromTLS(client, client->buf + client->buffered, RFB_BUF_SIZE - client->buffered);
-      else
-#ifdef LIBVNCSERVER_HAVE_SASL
-      if (client->saslconn)
-        i = ReadFromSASL(client, client->buf + client->buffered, RFB_BUF_SIZE - client->buffered);
-      else {
-#endif /* LIBVNCSERVER_HAVE_SASL */
-        i = read(client->sock, client->buf + client->buffered, RFB_BUF_SIZE - client->buffered);
-#ifdef LIBVNCSERVER_HAVE_SASL
-      }
-#endif
-  
-      if (i <= 0) {
-	if (i < 0) {
-	  if (errno == EWOULDBLOCK || errno == EAGAIN) {
-	    if (client->readTimeout > 0 &&
-		++retries > (client->readTimeout * 1000 * 1000 / USECS_WAIT_PER_RETRY))
-	    {
-	      rfbClientLog("Connection timed out\n");
-	      return FALSE;
-	    }
-	    /* TODO:
-	       ProcessXtEvents();
-	    */
-	    WaitForMessage(client, USECS_WAIT_PER_RETRY);
-	    i = 0;
-	  } else {
-	    rfbClientErr("read (%d: %s)\n",errno,strerror(errno));
-	    return FALSE;
-	  }
-	} else {
-	  if (errorMessageOnReadFailure) {
-	    rfbClientLog("VNC server closed connection\n");
-	  }
-	  return FALSE;
-	}
-      }
-      client->buffered += i;
-    }
-
-    memcpy(out, client->bufoutptr, n);
-    client->bufoutptr += n;
-    client->buffered -= n;
-
-  } else {
-
-    while (n > 0) {
-      int i;
-      if (client->tlsSession)
-        i = ReadFromTLS(client, out, n);
-      else
-#ifdef LIBVNCSERVER_HAVE_SASL
-      if (client->saslconn)
-        i = ReadFromSASL(client, out, n);
-      else
-#endif
-        i = read(client->sock, out, n);
-
-      if (i <= 0) {
-	if (i < 0) {
-	  if (errno == EWOULDBLOCK || errno == EAGAIN) {
-	    if (client->readTimeout > 0 &&
-		++retries > (client->readTimeout * 1000 * 1000 / USECS_WAIT_PER_RETRY))
-	    {
-		rfbClientLog("Connection timed out\n");
-		return FALSE;
-	    }
-	    /* TODO:
-	       ProcessXtEvents();
-	    */
-	    WaitForMessage(client, USECS_WAIT_PER_RETRY);
-	    i = 0;
-	  } else {
-	    rfbClientErr("read (%s)\n",strerror(errno));
-	    return FALSE;
-	  }
-	} else {
-	  if (errorMessageOnReadFailure) {
-	    rfbClientLog("VNC server closed connection\n");
-	  }
-	  return FALSE;
-	}
-      }
-      out += i;
-      n -= i;
-    }
-  }
-
-#ifdef DEBUG_READ_EXACT
-hexdump:
-  { unsigned int ii;
-    for(ii=0;ii<nn;ii++)
-      fprintf(stderr,"%02x ",(unsigned char)oout[ii]);
-    fprintf(stderr,"\n");
-  }
-#endif
-
 	return TRUE;
 }
 
-
 /*
  * Write an exact number of bytes, and don't return until you've sent them.
  */
-
 rfbBool
 WriteToRFBServer(rfbClient* client, const char *buf, unsigned int n)
 {

src/vnc.c

@@ -47,6 +47,21 @@ static uint64_t vnc_client_htonll(uint64_t x)
 #endif
 }
 
+static bool vnc_client_lock_handler(struct vnc_client* self)
+{
+	if (self->handler_lock)
+		return false;
+
+	self->handler_lock = true;
+	return true;
+}
+
+static void vnc_client_unlock_handler(struct vnc_client* self)
+{
+	assert(self->handler_lock);
+	self->handler_lock = false;
+}
+
 static rfbBool vnc_client_alloc_fb(rfbClient* client)
 {
 	struct vnc_client* self = rfbClientGetClientData(client, NULL);
@@ -228,20 +243,27 @@ int vnc_client_connect(struct vnc_client* self, const char* address, int port)
 {
 	rfbClient* client = self->client;
 
-	if (!ConnectToRFBServer(client, address, port))
+	return ConnectToRFBServer(client, address, port) ? 0 : -1;
-		return -1;
+}
+
+int vnc_client_init(struct vnc_client* self)
+{
+	int rc = -1;
+	rfbClient* client = self->client;
+
+	vnc_client_lock_handler(self);
 
 	if (!InitialiseRFBConnection(client))
-		return -1;
+		goto failure;
 
 	client->width = client->si.framebufferWidth;
 	client->height = client->si.framebufferHeight;
 
 	if (!client->MallocFrameBuffer(client))
-		return -1;
+		goto failure;
 
 	if (!vnc_client_set_format_and_encodings(client))
-		return -1;
+		goto failure;
 
 	if (client->updateRect.x < 0) {
 		client->updateRect.x = client->updateRect.y = 0;
@@ -253,12 +275,15 @@ int vnc_client_connect(struct vnc_client* self, const char* address, int port)
 				client->updateRect.x, client->updateRect.y,
 				client->updateRect.w, client->updateRect.h,
 				FALSE))
-		return -1;
+		goto failure;
 
 	SendIncrementalFramebufferUpdateRequest(client);
 	SendIncrementalFramebufferUpdateRequest(client);
 
-	return 0;
+	rc = 0;
+failure:
+	vnc_client_unlock_handler(self);
+	return rc;
 }
 
 int vnc_client_set_pixel_format(struct vnc_client* self, uint32_t format)
@@ -335,7 +360,14 @@ const char* vnc_client_get_desktop_name(const struct vnc_client* self)
 
 int vnc_client_process(struct vnc_client* self)
 {
-	return HandleRFBServerMessage(self->client) ? 0 : -1;
+	ReadToBuffer(self->client);
+
+	if (!vnc_client_lock_handler(self))
+		return 0;
+
+	int rc = HandleRFBServerMessage(self->client) ? 0 : -1;
+	vnc_client_unlock_handler(self);
+	return rc;
 }
 
 void vnc_client_send_pointer_event(struct vnc_client* self, int x, int y,