From 881bf98877d8a2dcaa9c246ab69153c5d03af748 Mon Sep 17 00:00:00 2001
From: NRK <nrk@disroot.org>
Date: Mon, 24 Jun 2024 07:04:30 +0000
Subject: [PATCH] fix racy --freeze behavior

this patch makes a couple different changes:

* previously the grab/ungrab was happening inside of
  selection_classic::{create,destroy}. this meant that
  `--freeze` was ineffective when edge mode was used.
  the patch moves those calls outside of it, so that `--freeze`
  can work on edge mode as well.
* because the grab/ungrab was happening inside selection
  destroy, there were multiple races. one of them was that the
  server would be ungrabbed after selection was done but
  *before* the shot was captured. the sleep introduced in commit
  93e475d widened the race window and made it more easily
  noticeable. the patch makes it so that we don't ungrab the
  server until the shot is actually taken.

Fixes: https://github.com/resurrecting-open-source-projects/scrot/issues/381
---
 src/scrot.c             | 12 ++++++------
 src/scrot.h             |  2 +-
 src/scrot_selection.c   | 27 +++++++++++++++++++--------
 src/selection_classic.c |  6 ------
 4 files changed, 26 insertions(+), 21 deletions(-)

diff --git a/src/scrot.c b/src/scrot.c
index 616004b..b621d63 100644
--- a/src/scrot.c
+++ b/src/scrot.c
@@ -297,9 +297,9 @@ size_t scrotHaveFileExtension(const char *filename, char **ext)
     return strlen(*ext);
 }
 
-Imlib_Image scrotGrabRectAndPointer(int x, int y, int w, int h)
+Imlib_Image scrotGrabRectAndPointer(int x, int y, int w, int h, bool isGrabbed)
 {
-    Imlib_Image im = imlib_create_image_from_drawable(0, x, y, w, h, 1);
+    Imlib_Image im = imlib_create_image_from_drawable(0, x, y, w, h, !isGrabbed);
     if (!im)
         errx(EXIT_FAILURE, "failed to grab image");
     if (opt.pointer)
@@ -315,7 +315,7 @@ static Imlib_Image scrotGrabWindowById(Window const window)
     if (!scrotGetGeometry(window, &rx, &ry, &rw, &rh))
         return NULL;
     scrotNiceClip(&rx, &ry, &rw, &rh);
-    im = scrotGrabRectAndPointer(rx, ry, rw, rh);
+    im = scrotGrabRectAndPointer(rx, ry, rw, rh, false);
     clientWindow = window;
     return im;
 }
@@ -333,7 +333,7 @@ static Imlib_Image scrotGrabAutoselect(void)
     int rx = opt.autoselectX, ry = opt.autoselectY, rw = opt.autoselectW,
         rh = opt.autoselectH;
     scrotNiceClip(&rx, &ry, &rw, &rh);
-    return scrotGrabRectAndPointer(rx, ry, rw, rh);
+    return scrotGrabRectAndPointer(rx, ry, rw, rh, false);
 }
 
 void scrotDoDelay(void)
@@ -625,7 +625,7 @@ static Imlib_Image scrotGrabShot(void)
 {
     if (!opt.silent)
         XBell(disp, 0);
-    return scrotGrabRectAndPointer(0, 0, scr->width, scr->height);
+    return scrotGrabRectAndPointer(0, 0, scr->width, scr->height, false);
 }
 
 static void scrotExecApp(Imlib_Image image, struct tm *tm, char *filenameIM,
@@ -984,7 +984,7 @@ static Imlib_Image scrotGrabShotMonitor(void)
     XFree(screens);
 
     scrotNiceClip(&x, &y, &w, &h);
-    return scrotGrabRectAndPointer(x, y, w, h);
+    return scrotGrabRectAndPointer(x, y, w, h, false);
 }
 
 static Imlib_Image stalkImageConcat(
diff --git a/src/scrot.h b/src/scrot.h
index 97ef37c..ea19619 100644
--- a/src/scrot.h
+++ b/src/scrot.h
@@ -46,7 +46,7 @@ void scrotNiceClip(int *, int *, int *, int *);
 struct timespec clockNow(void);
 struct timespec scrotSleepFor(struct timespec, int);
 void scrotDoDelay(void);
-Imlib_Image scrotGrabRectAndPointer(int x, int y, int w, int h);
+Imlib_Image scrotGrabRectAndPointer(int, int, int, int, bool);
 size_t scrotHaveFileExtension(const char *, char **);
 
 #endif /* !defined(H_SCROT) */
diff --git a/src/scrot_selection.c b/src/scrot_selection.c
index 9e00df0..9483e9c 100644
--- a/src/scrot_selection.c
+++ b/src/scrot_selection.c
@@ -413,22 +413,33 @@ Imlib_Image scrotSelectionSelectMode(void)
     if (opt.delaySelection)
         scrotDoDelay();
 
-    if (!scrotSelectionGetUserSel(&rect0))
-        return NULL;
-
-    opt.selection.mode = oldMode;
+    if (opt.freeze)
+        XGrabServer(disp);
+
+    bool success = scrotSelectionGetUserSel(&rect0);
+    if (success) {
+        opt.selection.mode = oldMode;
+        if (opt.selection.mode & SELECTION_MODE_NOT_CAPTURE)
+            success = scrotSelectionGetUserSel(&rect1);
+    }
 
-    if (opt.selection.mode & SELECTION_MODE_NOT_CAPTURE)
-        if (!scrotSelectionGetUserSel(&rect1))
-            return NULL;
+    if (!success) {
+        if (opt.freeze)
+            XUngrabServer(disp);
+        return NULL;
+    }
 
+    // this doesn't seem to make much sense if `--freeze` is enabled...
     if (!opt.delaySelection) {
         opt.delayStart = clockNow();
         scrotDoDelay();
     }
 
     Imlib_Image capture = scrotGrabRectAndPointer(
-        rect0.x, rect0.y, rect0.w, rect0.h);
+        rect0.x, rect0.y, rect0.w, rect0.h, opt.freeze);
+
+    if (opt.freeze)
+        XUngrabServer(disp);
 
     if (opt.selection.mode == SELECTION_MODE_CAPTURE)
         return capture;
diff --git a/src/selection_classic.c b/src/selection_classic.c
index 7eaa532..0a90dd2 100644
--- a/src/selection_classic.c
+++ b/src/selection_classic.c
@@ -71,9 +71,6 @@ void selectionClassicCreate(void)
 
     XSetLineAttributes(disp, pc->gc, opt.lineWidth, opt.lineStyle, CapRound,
         JoinRound);
-
-    if (opt.freeze)
-        XGrabServer(disp);
 }
 
 void selectionClassicDraw(void)
@@ -100,9 +97,6 @@ void selectionClassicDestroy(void)
     const struct Selection *const sel = &selection;
     const struct SelectionClassic *pc = &sel->classic;
 
-    if (opt.freeze)
-        XUngrabServer(disp);
-
     if (pc->gc)
         XFreeGC(disp, pc->gc);