Order macOS pointer events by the previous button state, scale the cursor position into framebuffer pixels, and escalate crash cleanup to SIGKILL

client/vnc/server/capture_darwin.go

@@ -111,6 +111,11 @@ type CGCapturer struct {
 	w, h      int
 	// downscale is 1 for pixel-perfect, 2 for Retina 2:1 box-filter downscale.
 	downscale int
+	// logicalW/logicalH are the display's size in logical points, the unit
+	// CGEventGetLocation reports the cursor in. Kept so CursorPos can convert
+	// into the framebuffer's own pixel grid, which differs from it whenever
+	// the display is Retina.
+	logicalW, logicalH int
 	hashSeed  maphash.Seed
 	lastHash  uint64
 	hasHash   bool
@@ -197,6 +202,7 @@ func NewCGCapturer() (*CGCapturer, error) {
 	}
 	c.w = nativeW / c.downscale
 	c.h = nativeH / c.downscale
+	c.logicalW, c.logicalH = logicalW, logicalH
 
 	log.Infof("macOS capturer ready: %dx%d (native %dx%d, logical %dx%d, downscale=%d, display=%d)",
 		c.w, c.h, nativeW, nativeH, logicalW, logicalH, c.downscale, displayID)

client/vnc/server/cursor_darwin.go

@@ -156,8 +156,14 @@ func (c *CGCapturer) Cursor() (*image.RGBA, int, int, uint64, error) {
 	return c.cursor.Cursor()
 }
 
-// CursorPos returns the current global mouse location via CGEventCreate /
-// CGEventGetLocation. Coordinates are screen pixels in the main display.
+// CursorPos returns the current mouse location in this capturer's framebuffer
+// pixels, which is the space the caller composites the sprite into.
+//
+// CGEventGetLocation reports logical points. That is not the framebuffer's grid
+// on a Retina display: the frame is captured at native resolution and then
+// optionally halved, so the two differ by the display's backing scale factor
+// over downscale. Returning points unconverted puts the cursor at a fraction of
+// its real position whenever those disagree.
 func (c *CGCapturer) CursorPos() (int, int, error) {
 	if cgEventCreate == nil || cgEventGetLocation == nil {
 		return 0, 0, fmt.Errorf("CGEvent location APIs unavailable")
@@ -168,7 +174,11 @@ func (c *CGCapturer) CursorPos() (int, int, error) {
 	}
 	defer cfRelease(ev)
 	pt := cgEventGetLocation(ev)
-	return int(pt.X), int(pt.Y), nil
+
+	if c.logicalW <= 0 || c.logicalH <= 0 {
+		return int(pt.X), int(pt.Y), nil
+	}
+	return int(pt.X) * c.w / c.logicalW, int(pt.Y) * c.h / c.logicalH, nil
 }
 
 // Cursor on MacPoller forwards to the lazy CGCapturer. ensureCapturerLocked

client/vnc/server/input_darwin.go

@@ -706,13 +706,15 @@ func scalePxToLogical(px, py, serverW, serverH int) (float64, float64) {
 }
 
 func (m *MacInputInjector) dispatchPointer(src uintptr, buttonMask uint16, x, y float64) {
-	leftDown := buttonMask&0x01 != 0
-	rightDown := buttonMask&0x04 != 0
-	middleDown := buttonMask&0x02 != 0
-	m.postMoveOrDrag(src, leftDown, rightDown, x, y)
+	// Move with the buttons that were already held, not the ones this event
+	// introduces. Using the new state posts the motion as a drag before the
+	// mouse-down that begins it, and as a plain move before the mouse-up that
+	// ends it, so a press reads as a drag with no click and a release drops the
+	// drag one event early.
+	prev := m.lastButtons
+	m.postMoveOrDrag(src, prev&0x01 != 0, prev&0x04 != 0, x, y)
 	m.postButtonTransitions(src, buttonMask, x, y)
 	m.postScrollWheel(src, buttonMask)
-	_ = middleDown
 }
 
 func (m *MacInputInjector) postMoveOrDrag(src uintptr, leftDown, rightDown bool, x, y float64) {

client/vnc/server/shutdown_state.go

@@ -9,6 +9,7 @@ import (
 	"strconv"
 	"strings"
 	"syscall"
+	"time"
 
 	log "github.com/sirupsen/logrus"
 )
@@ -61,6 +62,19 @@ func (s *ShutdownState) Cleanup() error {
 			if killErr := syscall.Kill(proc.PID, syscall.SIGKILL); killErr != nil {
 				log.Debugf("cleanup: kill pid %d (%s): group kill: %v, single kill: %v", proc.PID, desc, err, killErr)
 			}
+			continue
+		}
+
+		// An X server or a desktop process may catch or ignore TERM, and this
+		// record is discarded below either way, so nothing would come back for
+		// it. Escalate the way the ordinary virtual-session shutdown does
+		// rather than leaving it running against the next session.
+		if groupGone(proc.PID, cleanupGracePeriod) {
+			continue
+		}
+		log.Debugf("cleanup: pid %d (%s) survived SIGTERM, sending SIGKILL", proc.PID, desc)
+		if err := syscall.Kill(-proc.PID, syscall.SIGKILL); err != nil {
+			log.Debugf("cleanup: SIGKILL pid %d (%s): %v", proc.PID, desc, err)
 		}
 	}
 
@@ -68,6 +82,25 @@ func (s *ShutdownState) Cleanup() error {
 	return nil
 }
 
+// cleanupGracePeriod is how long a signalled process group gets to exit on its
+// own before Cleanup escalates to SIGKILL.
+const cleanupGracePeriod = 2 * time.Second
+
+// groupGone polls the process group until it has exited or grace expires, and
+// reports whether it is gone. Signal 0 only probes for existence.
+func groupGone(pid int, grace time.Duration) bool {
+	deadline := time.Now().Add(grace)
+	for {
+		if err := syscall.Kill(-pid, 0); err != nil {
+			return true
+		}
+		if !time.Now().Before(deadline) {
+			return false
+		}
+		time.Sleep(50 * time.Millisecond)
+	}
+}
+
 // describeProcess captures the identity of a freshly started process so a later
 // Cleanup can tell it apart from whatever inherits its PID.
 func describeProcess(pid int) sessionProcess {