Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 03 Sep 2019 14:05:58 -0000
From:      Bruce Evans <bde@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   svn commit: r345695 - head/lib/libvgl
Message-ID:  <201903291520.x2TFKm5e076140@repo.freebsd.org>

next in thread | raw e-mail | index | archive | help
Author: bde
Date: Fri Mar 29 15:20:48 2019
New Revision: 345695
URL: https://svnweb.freebsd.org/changeset/base/345695

Log:
  Fix races in mouse signal handling almost properly using the INTOFF/INTON
  method as in /bin/sh.
  
  We still do technically undefined things in the signal handler, but it
  is safe in practice to access state that is protected by INTOFF/INTON.
  
  In a recent commit, I sprinkled VGLMouseFrozen++/-- operations in
  places that need INTOFF/INTON.  This prevented clobbering of pixels
  under the mouse, but left mouse signals deferred for too long.  It is
  necessary to call the signal handler when the count goes to 0.  Old
  versions did this in the unfreeze function, but didn't block actual
  signals, so the signal handler raced itself.  The sprinkled operations
  reduced the races, but when then worked to block a race they left
  signals deferred for too long.
  
  Use INTOFF/INTON to fix complete loss of mouse signals while reading
  the mouse status.  Clobbering of the state was prevented by SIG_IGN'ing
  mouse signals, but that has a high overhead and broke more than it
  fixed by losing mouse signals completely.  sigprocmask() works to block
  signals without losing them completely, but its overhead is also too
  high.
  
  libvgl's mouse signal handling is often worse than none.  Applications
  can't block waiting for a mouse or keyboard or other event, but have
  to busy-wait.  The SIG_IGN's lost about half of all mouse events while
  busy-waiting for mouse events.

Modified:
  head/lib/libvgl/mouse.c

Modified: head/lib/libvgl/mouse.c
==============================================================================
--- head/lib/libvgl/mouse.c	Fri Mar 29 15:07:00 2019	(r345694)
+++ head/lib/libvgl/mouse.c	Fri Mar 29 15:20:48 2019	(r345695)
@@ -86,12 +86,19 @@ static byte map[MOUSE_IMG_SIZE*MOUSE_IMG_SIZE*4];
 static VGLBitmap VGLMouseSave = 
     VGLBITMAP_INITIALIZER(MEMBUF, MOUSE_IMG_SIZE, MOUSE_IMG_SIZE, map);
 static int VGLMouseVisible = 0;
-static int VGLMouseFrozen = 0;
 static int VGLMouseShown = 0;
 static int VGLMouseXpos = 0;
 static int VGLMouseYpos = 0;
 static int VGLMouseButtons = 0;
+static volatile sig_atomic_t VGLMintpending;
+static volatile sig_atomic_t VGLMsuppressint;
 
+#define	INTOFF()	(VGLMsuppressint++)
+#define	INTON()		do { 						\
+				if (--VGLMsuppressint == 0 && VGLMintpending) \
+					VGLMouseAction(0);		\
+			} while (0)
+
 void
 VGLMousePointerShow()
 {
@@ -102,7 +109,7 @@ VGLMousePointerShow()
   int i, pos, pos1;
 
   if (!VGLMouseVisible) {
-    VGLMouseFrozen++;
+    INTOFF();
     VGLMouseVisible = 1;
     crtcidx = inb(0x3c4);
     crtcval = inb(0x3c5);
@@ -125,7 +132,7 @@ VGLMousePointerShow()
     outb(0x3c5, crtcval);
     outb(0x3ce, gdcidx);
     outb(0x3cf, gdcval);
-    VGLMouseFrozen--;
+    INTON();
   }
 }
 
@@ -135,7 +142,7 @@ VGLMousePointerHide()
   byte crtcidx, crtcval, gdcidx, gdcval;
 
   if (VGLMouseVisible) {
-    VGLMouseFrozen++;
+    INTOFF();
     VGLMouseVisible = 0;
     crtcidx = inb(0x3c4);
     crtcval = inb(0x3c5);
@@ -147,7 +154,7 @@ VGLMousePointerHide()
     outb(0x3c5, crtcval);
     outb(0x3ce, gdcidx);
     outb(0x3cf, gdcval);
-    VGLMouseFrozen--;
+    INTON();
   }
 }
 
@@ -173,10 +180,13 @@ VGLMouseAction(int dummy)	
 {
   struct mouse_info mouseinfo;
 
-  if (VGLMouseFrozen) {
-    VGLMouseFrozen += 8;
+  if (VGLMsuppressint) {
+    VGLMintpending = 1;
     return;
   }
+again:
+  INTOFF();
+  VGLMintpending = 0;
   mouseinfo.operation = MOUSE_GETINFO;
   ioctl(0, CONS_MOUSECTL, &mouseinfo);
   if (VGLMouseShown == VGL_MOUSESHOW)
@@ -186,6 +196,15 @@ VGLMouseAction(int dummy)	
   VGLMouseButtons = mouseinfo.u.data.buttons;
   if (VGLMouseShown == VGL_MOUSESHOW)
     VGLMousePointerShow();
+
+  /* 
+   * Loop to handle any new (suppressed) signals.  This is INTON() without
+   * recursion.  !SA_RESTART prevents recursion in signal handling.  So the
+   * maximum recursion is 2 levels.
+   */
+  VGLMsuppressint = 0;
+  if (VGLMintpending)
+    goto again;
 }
 
 void
@@ -248,11 +267,11 @@ VGLMouseInit(int mode)
 int
 VGLMouseStatus(int *x, int *y, char *buttons)
 {
-  signal(SIGUSR2, SIG_IGN);
+  INTOFF();
   *x =  VGLMouseXpos;
   *y =  VGLMouseYpos;
   *buttons =  VGLMouseButtons;
-  signal(SIGUSR2, VGLMouseAction);
+  INTON();
   return VGLMouseShown;
 }
 
@@ -261,7 +280,7 @@ VGLMouseFreeze(int x, int y, int width, int hight, u_l
 {
   int i, xstride, ystride;
 
-    VGLMouseFrozen++;
+    INTOFF();
     if (width > 1 || hight > 1 || (color & 0xc0000000) == 0) { /* bitmap */
       if (VGLMouseShown == 1) {
         int overlap;
@@ -311,13 +330,8 @@ VGLMouseFreeze(int x, int y, int width, int hight, u_l
 void
 VGLMouseUnFreeze()
 {
-  if (VGLMouseFrozen > 8) {
-    VGLMouseFrozen = 0;
-    VGLMouseAction(0);
-  }
-  else {
-    if (VGLMouseShown == VGL_MOUSESHOW && !VGLMouseVisible)
-      VGLMousePointerShow();
-    VGLMouseFrozen = 0;
-  }
+  if (VGLMouseShown == VGL_MOUSESHOW && !VGLMouseVisible && !VGLMintpending)
+    VGLMousePointerShow();
+  while (VGLMsuppressint)
+    INTON();
 }





Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201903291520.x2TFKm5e076140>