Fix incorrect handling of 'do_action, close' mouse action (again!)

src/menu.c

@@ -23,12 +23,12 @@
 static bool is_initialized = false;
 static regex_t url_regex;
 
-struct notification_lock {
-        struct notification *n;
-        gint64 timeout;
-};
 static gpointer context_menu_thread(gpointer data);
 
+struct {
+        GList *locked_notifications;
+} menu_ctx;
+
 /**
  * Initializes regexes needed for matching.
  *
@@ -290,9 +290,37 @@ char *invoke_dmenu(const char *dmenu_input)
         return ret;
 }
 
+/**
+ * Lock and get all notifications with an action or URL.
+ **/
+static GList *get_actionable_notifications(void)
+{
+        GList *locked_notifications = NULL;
+
+        for (const GList *iter = queues_get_displayed(); iter;
+             iter = iter->next) {
+                struct notification *n = iter->data;
+
+                if (n->urls || g_hash_table_size(n->actions)) {
+                        notification_lock(n);
+                        locked_notifications = g_list_prepend(locked_notifications, n);
+                }
+        }
+
+        return locked_notifications;
+}
+
 /* see menu.h */
 void context_menu(void)
 {
+        if (menu_ctx.locked_notifications) {
+                LOG_W("Context menu already running, refusing to rerun");
+                return;
+        }
+
+        GList *notifications = get_actionable_notifications();
+        menu_ctx.locked_notifications = notifications;
+
         GError *err = NULL;
         g_thread_unref(g_thread_try_new("dmenu",
                                         context_menu_thread,
@@ -305,32 +333,41 @@ void context_menu(void)
         }
 }
 
+static gboolean context_menu_result_dispatch(gpointer user_data)
+{
+        char *dmenu_output = (char*)user_data;
+
+        dispatch_menu_result(dmenu_output);
+
+        for (GList *iter = menu_ctx.locked_notifications; iter; iter = iter->next) {
+                struct notification *n = iter->data;
+                notification_unlock(n);
+                if (n->marked_for_closure) {
+                        // Don't close notification if context was aborted
+                        if (dmenu_output != NULL)
+                                queues_notification_close(n, n->marked_for_closure);
+                        n->marked_for_closure = 0;
+                }
+        }
+
+        menu_ctx.locked_notifications = NULL;
+
+        g_list_free(menu_ctx.locked_notifications);
+        g_free(dmenu_output);
+
+        wake_up();
+
+        return G_SOURCE_REMOVE;
+}
+
 static gpointer context_menu_thread(gpointer data)
 {
         char *dmenu_input = NULL;
         char *dmenu_output;
 
-        GList *locked_notifications = NULL;
+        for (GList *iter = menu_ctx.locked_notifications; iter; iter = iter->next) {
-
-        for (const GList *iter = queues_get_displayed(); iter;
-             iter = iter->next) {
                 struct notification *n = iter->data;
 
-
-                // Reference and lock the notification if we need it
-                if (n->urls || g_hash_table_size(n->actions)) {
-                        notification_ref(n);
-
-                        struct notification_lock *nl =
-                                g_malloc(sizeof(struct notification_lock));
-
-                        nl->n = n;
-                        nl->timeout = n->timeout;
-                        n->timeout = 0;
-
-                        locked_notifications = g_list_prepend(locked_notifications, nl);
-                }
-
                 char *dmenu_str = notification_dmenu_string(n);
                 dmenu_input = string_append(dmenu_input, dmenu_str, "\n");
                 g_free(dmenu_str);
@@ -340,25 +377,9 @@ static gpointer context_menu_thread(gpointer data)
         }
 
         dmenu_output = invoke_dmenu(dmenu_input);
-        dispatch_menu_result(dmenu_output);
+        g_timeout_add(50, context_menu_result_dispatch, dmenu_output);
 
         g_free(dmenu_input);
-        g_free(dmenu_output);
-
-        // unref all notifications
-        for (GList *iter = locked_notifications;
-                    iter;
-                    iter = iter->next) {
-
-                struct notification_lock *nl = iter->data;
-                struct notification *n = nl->n;
-
-                n->timeout = nl->timeout;
-
-                g_free(nl);
-                notification_unref(n);
-        }
-        g_list_free(locked_notifications);
 
         return NULL;
 }

src/notification.c

@@ -198,6 +198,30 @@ bool notification_is_duplicate(const struct notification *a, const struct notifi
             && a->urgency == b->urgency;
 }
 
+bool notification_is_locked(struct notification *n) {
+        assert(n);
+
+        return g_atomic_int_get(&n->locked) != 0;
+}
+
+struct notification* notification_lock(struct notification *n) {
+        assert(n);
+
+        g_atomic_int_set(&n->locked, 1);
+        notification_ref(n);
+
+        return n;
+}
+
+struct notification* notification_unlock(struct notification *n) {
+        assert(n);
+
+        g_atomic_int_set(&n->locked, 0);
+        notification_unref(n);
+
+        return n;
+}
+
 static void notification_private_free(NotificationPrivate *p)
 {
         g_free(p);

src/notification.h

@@ -58,6 +58,7 @@ struct notification {
         gint64 start;      /**< begin of current display */
         gint64 timestamp;  /**< arrival time */
         gint64 timeout;    /**< time to display */
+        int locked;     /**< If non-zero the notification is locked **/
 
         GHashTable *actions;
 
@@ -82,6 +83,7 @@ struct notification {
         int displayed_height;
         enum behavior_fullscreen fullscreen; //!< The instruction what to do with it, when desktop enters fullscreen
         bool script_run;        /**< Has the script been executed already? */
+        guint8 marked_for_closure;
 
         /* derived fields */
         char *msg;            /**< formatted message */
@@ -139,6 +141,12 @@ int notification_cmp_data(const void *va, const void *vb, void *data);
 
 bool notification_is_duplicate(const struct notification *a, const struct notification *b);
 
+bool notification_is_locked(struct notification *n);
+
+struct notification *notification_lock(struct notification *n);
+
+struct notification *notification_unlock(struct notification *n);
+
 /**Replace the current notification's icon with the icon specified by path.
  *
  * Removes the reference for the previous icon automatically and will also free the

src/queues.c

@@ -391,10 +391,20 @@ void queues_update(struct dunst_status status)
                 struct notification *n = iter->data;
                 nextiter = iter->next;
 
+                if (notification_is_locked(n)) {
+                        iter = nextiter;
+                        continue;
+                }
+
                 if (queues_notification_is_finished(n, status)){
                         queues_notification_close(n, REASON_TIME);
                         iter = nextiter;
                         continue;
+                } else if (n->marked_for_closure) {
+                        queues_notification_close(n, n->marked_for_closure);
+                        n->marked_for_closure = 0;
+                        iter = nextiter;
+                        continue;
                 }
 
                 if (!queues_notification_is_ready(n, status, true)) {

src/x11/x.c

@@ -450,19 +450,15 @@ static void x_handle_click(XEvent ev)
 
                         if (n) {
                                 if (act == MOUSE_CLOSE_CURRENT) {
-                                        // We cannot call
+                                        n->marked_for_closure = REASON_USER;
-                                        // queues_close_notification here as
-                                        // the do_action runs in a separate
-                                        // thread, so force expire the
-                                        // notification instead
-                                        n->timeout = 1;
-                                        n->start -= 1;
                                 } else {
                                         notification_do_action(n);
                                 }
                         }
                 }
         }
+
+        wake_up();
 }
 
 void x_free(void)