Date: Fri, 29 Mar 2019 15:20:48 +0000 (UTC) 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>