Rework X selections to make it safe to run the debugger inside converters

* src/keyboard.c (prev_kbd_event): Delete function.
(readable_events): Return 1 if
x_detect_pending_selection_requests returns true.
(kbd_buffer_unget_event): Also delete function, since nested
selection requests are really handled correctly.
(kbd_buffer_get_event): Handle events from the special X
deferred selection queue as well.

* src/keyboard.h: Update prototypes.

* src/xselect.c (struct selection_event_queue)
(selection_input_event_equal, x_queue_event)
(x_start_queuing_selection_requests)
(x_stop_queuing_selection_requests): Delete structs, since they
are no longer required.
(x_handle_selection_request, x_handle_selection_event): Allow
nested selection events.

* src/xterm.c (struct x_selection_request_event): New struct.
(x_handle_pending_selection_requests_1)
(x_handle_pending_selection_requests): Handle all events in the
new selection event queue.
(x_push_selection_request, x_detect_pending_selection_requests):
New functions.
(x_dnd_begin_drag_and_drop): Drain the selection queue here as
well.
(handle_one_xevent): When inside a nested event loop, just push
selections to that queue.
(XTread_socket): Allow reading X events if x_dnd_unwind_flag is
true, even though DND is in progress.
(x_delete_display): Delete pending selection events for the
display that is going away.

* src/xterm.h: Update prototypes.

5 files changed, 163 insertions, 162 deletions

diff --git a/src/keyboard.c b/src/keyboard.c
index 274c7b3fa84..55d710ed627 100644
--- a/src/keyboard.c
+++ b/src/keyboard.c
@@ -389,14 +389,6 @@ next_kbd_event (union buffered_input_event *ptr)
   return ptr == kbd_buffer + KBD_BUFFER_SIZE - 1 ? kbd_buffer : ptr + 1;
 }
 
-#ifdef HAVE_X11
-static union buffered_input_event *
-prev_kbd_event (union buffered_input_event *ptr)
-{
-  return ptr == kbd_buffer ? kbd_buffer + KBD_BUFFER_SIZE - 1 : ptr - 1;
-}
-#endif
-
 /* Like EVENT_START, but assume EVENT is an event.
    This pacifies gcc -Wnull-dereference, which might otherwise
    complain about earlier checks that EVENT is indeed an event.  */
@@ -3528,6 +3520,11 @@ readable_events (int flags)
         return 1;
     }
 
+#ifdef HAVE_X_WINDOWS
+  if (x_detect_pending_selection_requests ())
+    return 1;
+#endif
+
   if (!(flags & READABLE_EVENTS_IGNORE_SQUEEZABLES) && some_mouse_moved ())
     return 1;
   if (single_kboard)
@@ -3699,25 +3696,6 @@ kbd_buffer_store_buffered_event (union buffered_input_event *event,
     Vquit_flag = Vthrow_on_input;
 }
 
-
-#ifdef HAVE_X11
-
-/* Put a selection input event back in the head of the event queue.  */
-
-void
-kbd_buffer_unget_event (struct selection_input_event *event)
-{
-  /* Don't let the very last slot in the buffer become full,  */
-  union buffered_input_event *kp = prev_kbd_event (kbd_fetch_ptr);
-  if (kp != kbd_store_ptr)
-    {
-      kp->sie = *event;
-      kbd_fetch_ptr = kp;
-    }
-}
-
-#endif
-
 /* Limit help event positions to this range, to avoid overflow problems.  */
 #define INPUT_EVENT_POS_MAX \
   ((ptrdiff_t) min (PTRDIFF_MAX, min (TYPE_MAXIMUM (Time) / 2, \
@@ -3874,6 +3852,11 @@ kbd_buffer_get_event (KBOARD **kbp,
                       struct timespec *end_time)
 {
   Lisp_Object obj, str;
+#ifdef HAVE_X_WINDOWS
+  bool had_pending_selection_requests;
+
+  had_pending_selection_requests = false;
+#endif
 
 #ifdef subprocesses
   if (kbd_on_hold_p () && kbd_buffer_nr_stored () < KBD_BUFFER_SIZE / 4)
@@ -3926,10 +3909,18 @@ kbd_buffer_get_event (KBOARD **kbp,
 #if defined (USABLE_SIGIO) || defined (USABLE_SIGPOLL)
       gobble_input ();
 #endif
+
       if (kbd_fetch_ptr != kbd_store_ptr)
         break;
       if (some_mouse_moved ())
         break;
+#ifdef HAVE_X_WINDOWS
+      if (x_detect_pending_selection_requests ())
+        {
+          had_pending_selection_requests = true;
+          break;
+        }
+#endif
       if (end_time)
         {
           struct timespec now = current_timespec ();
@@ -3966,6 +3957,16 @@ kbd_buffer_get_event (KBOARD **kbp,
         gobble_input ();
     }
 
+#ifdef HAVE_X_WINDOWS
+  /* Handle pending selection requests.  This can happen if Emacs
+     enters a recursive edit inside a nested event loop (probably
+     because the debugger opened) or someone called
+     `read-char'.  */
+
+  if (had_pending_selection_requests)
+    x_handle_pending_selection_requests ();
+#endif
+
   if (CONSP (Vunread_command_events))
     {
       Lisp_Object first;
@@ -4345,6 +4346,10 @@ kbd_buffer_get_event (KBOARD **kbp,
                               ? movement_frame->last_mouse_device
                               : virtual_core_pointer_name);
     }
+#ifdef HAVE_X_WINDOWS
+  else if (had_pending_selection_requests)
+    obj = Qnil;
+#endif
   else
     /* We were promised by the above while loop that there was
        something for us to read!  */
@@ -7241,7 +7246,10 @@ lucid_event_type_list_p (Lisp_Object object)
    If READABLE_EVENTS_FILTER_EVENTS is set in FLAGS, ignore internal
    events (FOCUS_IN_EVENT).
    If READABLE_EVENTS_IGNORE_SQUEEZABLES is set in FLAGS, ignore mouse
-   movements and toolkit scroll bar thumb drags.  */
+   movements and toolkit scroll bar thumb drags.
+
+   On X, this also returns if the selection event chain is full, since
+   that's also "keyboard input".  */
 
 static bool
 get_input_pending (int flags)
diff --git a/src/keyboard.h b/src/keyboard.h
index a0b7204fa2b..6ae2dc9c4c6 100644
--- a/src/keyboard.h
+++ b/src/keyboard.h
@@ -481,9 +481,6 @@ kbd_buffer_store_event_hold (struct input_event *event,
   kbd_buffer_store_buffered_event ((union buffered_input_event *) event,
                                    hold_quit);
 }
-#ifdef HAVE_X11
-extern void kbd_buffer_unget_event (struct selection_input_event *);
-#endif
 extern void poll_for_input_1 (void);
 extern void show_help_echo (Lisp_Object, Lisp_Object, Lisp_Object,
                             Lisp_Object);
diff --git a/src/xselect.c b/src/xselect.c
index 5f2a0cf56de..6e693c25884 100644
--- a/src/xselect.c
+++ b/src/xselect.c
@@ -116,92 +116,6 @@ selection_quantum (Display *display)
   assq_no_quit (selection_symbol, dpyinfo->terminal->Vselection_alist)
 
 
-/* Define a queue to save up SELECTION_REQUEST_EVENT events for later
-   handling.  */
-
-struct selection_event_queue
-  {
-    struct selection_input_event event;
-    struct selection_event_queue *next;
-  };
-
-static struct selection_event_queue *selection_queue;
-
-/* Nonzero means queue up SELECTION_REQUEST_EVENT events.  */
-
-static int x_queue_selection_requests;
-
-/* True if the input events are duplicates.  */
-
-static bool
-selection_input_event_equal (struct selection_input_event *a,
-                             struct selection_input_event *b)
-{
-  return (a->kind == b->kind && a->dpyinfo == b->dpyinfo
-          && a->requestor == b->requestor && a->selection == b->selection
-          && a->target == b->target && a->property == b->property
-          && a->time == b->time);
-}
-
-/* Queue up an SELECTION_REQUEST_EVENT *EVENT, to be processed later.  */
-
-static void
-x_queue_event (struct selection_input_event *event)
-{
-  struct selection_event_queue *queue_tmp;
-
-  /* Don't queue repeated requests.
-     This only happens for large requests which uses the incremental protocol.  */
-  for (queue_tmp = selection_queue; queue_tmp; queue_tmp = queue_tmp->next)
-    {
-      if (selection_input_event_equal (event, &queue_tmp->event))
-        {
-          TRACE1 ("DECLINE DUP SELECTION EVENT %p", queue_tmp);
-          x_decline_selection_request (event);
-          return;
-        }
-    }
-
-  queue_tmp = xmalloc (sizeof *queue_tmp);
-  TRACE1 ("QUEUE SELECTION EVENT %p", queue_tmp);
-  queue_tmp->event = *event;
-  queue_tmp->next = selection_queue;
-  selection_queue = queue_tmp;
-}
-
-/* Start queuing SELECTION_REQUEST_EVENT events.  */
-
-static void
-x_start_queuing_selection_requests (void)
-{
-  if (x_queue_selection_requests)
-    emacs_abort ();
-
-  x_queue_selection_requests++;
-  TRACE1 ("x_start_queuing_selection_requests %d", x_queue_selection_requests);
-}
-
-/* Stop queuing SELECTION_REQUEST_EVENT events.  */
-
-static void
-x_stop_queuing_selection_requests (void)
-{
-  TRACE1 ("x_stop_queuing_selection_requests %d", x_queue_selection_requests);
-  --x_queue_selection_requests;
-
-  /* Take all the queued events and put them back
-     so that they get processed afresh.  */
-
-  while (selection_queue != NULL)
-    {
-      struct selection_event_queue *queue_tmp = selection_queue;
-      TRACE1 ("RESTORE SELECTION EVENT %p", queue_tmp);
-      kbd_buffer_unget_event (&queue_tmp->event);
-      selection_queue = queue_tmp->next;
-      xfree (queue_tmp);
-    }
-}
-
 
 /* This converts a Lisp symbol to a server Atom, avoiding a server
    roundtrip whenever possible.  */
@@ -839,11 +753,6 @@ x_handle_selection_request (struct selection_input_event *event)
   selection_request_dpyinfo = dpyinfo;
   record_unwind_protect_void (x_selection_request_lisp_error);
 
-  /* We might be able to handle nested x_handle_selection_requests,
-     but this is difficult to test, and seems unimportant.  */
-  x_start_queuing_selection_requests ();
-  record_unwind_protect_void (x_stop_queuing_selection_requests);
-
   TRACE2 ("x_handle_selection_request: selection=%s, target=%s",
           SDATA (SYMBOL_NAME (selection_symbol)),
           SDATA (SYMBOL_NAME (target_symbol)));
@@ -1028,8 +937,6 @@ x_handle_selection_event (struct selection_input_event *event)
   TRACE0 ("x_handle_selection_event");
   if (event->kind != SELECTION_REQUEST_EVENT)
     x_handle_selection_clear (event);
-  else if (x_queue_selection_requests)
-    x_queue_event (event);
   else
     x_handle_selection_request (event);
 }
diff --git a/src/xterm.c b/src/xterm.c
index 4d8d7e80eb5..ffbd09d27f0 100644
--- a/src/xterm.c
+++ b/src/xterm.c
@@ -789,6 +789,21 @@ static int current_finish;
 static struct input_event *current_hold_quit;
 #endif
 
+struct x_selection_request_event
+{
+  /* The selection request event.  */
+  struct selection_input_event se;
+
+  /* The next unprocessed selection request event.  */
+  struct x_selection_request_event *next;
+};
+
+/* Chain of unprocessed selection request events.  Used to handle
+   selection requests inside long-lasting modal event loops, such as
+   the drag-and-drop loop.  */
+
+struct x_selection_request_event *pending_selection_requests;
+
 /* Compare two request serials A and B with OP, handling
    wraparound.  */
 #define X_COMPARE_SERIALS(a, op ,b) \
@@ -1169,6 +1184,10 @@ static unsigned int x_dnd_keyboard_state;
    terminating DND as part of the display disconnect handler.  */
 static sigjmp_buf x_dnd_disconnect_handler;
 
+/* Whether or not the current invocation of handle_one_xevent
+   happened inside the drag_and_drop event loop.  */
+static bool x_dnd_inside_handle_one_xevent;
+
 /* Structure describing a single window that can be the target of
    drag-and-drop operations.  */
 struct x_client_list_window
@@ -10546,6 +10565,53 @@ x_next_event_from_any_display (XEvent *event)
 
 #endif /* USE_X_TOOLKIT || USE_GTK */
 
+static void
+x_handle_pending_selection_requests_1 (struct x_selection_request_event *tem)
+{
+  specpdl_ref count;
+  struct selection_input_event se;
+
+  count = SPECPDL_INDEX ();
+  se = tem->se;
+
+  record_unwind_protect_ptr (xfree, tem);
+  x_handle_selection_event (&se);
+  unbind_to (count, Qnil);
+}
+
+/* Handle all pending selection request events from modal event
+   loops.  */
+void
+x_handle_pending_selection_requests (void)
+{
+  struct x_selection_request_event *tem;
+
+  while (pending_selection_requests)
+    {
+      tem = pending_selection_requests;
+      pending_selection_requests = tem->next;
+
+      x_handle_pending_selection_requests_1 (tem);
+    }
+}
+
+static void
+x_push_selection_request (struct selection_input_event *se)
+{
+  struct x_selection_request_event *tem;
+
+  tem = xmalloc (sizeof *tem);
+  tem->next = pending_selection_requests;
+  tem->se = *se;
+  pending_selection_requests = tem;
+}
+
+bool
+x_detect_pending_selection_requests (void)
+{
+  return pending_selection_requests;
+}
+
 /* This function is defined far away from the rest of the XDND code so
    it can utilize `x_any_window_to_frame'.  */
 
@@ -10841,6 +10907,7 @@ x_dnd_begin_drag_and_drop (struct frame *f, Time time, Atom xaction,
   while (x_dnd_in_progress || x_dnd_waiting_for_finish)
     {
       EVENT_INIT (hold_quit);
+
 #ifdef USE_GTK
       current_finish = X_EVENT_NORMAL;
       current_hold_quit = &hold_quit;
@@ -10849,6 +10916,7 @@ x_dnd_begin_drag_and_drop (struct frame *f, Time time, Atom xaction,
 #endif
 
       block_input ();
+      x_dnd_inside_handle_one_xevent = true;
 #ifdef USE_GTK
       gtk_main_iteration ();
 #elif defined USE_X_TOOLKIT
@@ -10890,6 +10958,7 @@ x_dnd_begin_drag_and_drop (struct frame *f, Time time, Atom xaction,
       current_count = -1;
       current_hold_quit = NULL;
 #endif
+      x_dnd_inside_handle_one_xevent = false;
 
       /* The unblock_input below might try to read input, but
          XTread_socket does nothing inside a drag-and-drop event
@@ -10942,29 +11011,6 @@ x_dnd_begin_drag_and_drop (struct frame *f, Time time, Atom xaction,
 
           if (hold_quit.kind != NO_EVENT)
             {
-              if (hold_quit.kind == SELECTION_REQUEST_EVENT)
-                {
-                  /* It's not safe to run Lisp inside this function if
-                     x_dnd_in_progress and x_dnd_waiting_for_finish
-                     are unset, so push it back into the event queue.  */
-
-                  if (!x_dnd_in_progress && !x_dnd_waiting_for_finish)
-                    kbd_buffer_store_event (&hold_quit);
-                  else
-                    {
-                      x_dnd_old_window_attrs = root_window_attrs;
-                      x_dnd_unwind_flag = true;
-
-                      ref = SPECPDL_INDEX ();
-                      record_unwind_protect_ptr (x_dnd_cleanup_drag_and_drop, f);
-                      x_handle_selection_event ((struct selection_input_event *) &hold_quit);
-                      x_dnd_unwind_flag = false;
-                      unbind_to (ref, Qnil);
-                    }
-
-                  continue;
-                }
-
               if (x_dnd_in_progress)
                 {
                   if (x_dnd_last_seen_window != None
@@ -11031,6 +11077,19 @@ x_dnd_begin_drag_and_drop (struct frame *f, Time time, Atom xaction,
               quit ();
             }
 
+          if (pending_selection_requests
+              && (x_dnd_in_progress || x_dnd_waiting_for_finish))
+            {
+              x_dnd_old_window_attrs = root_window_attrs;
+              x_dnd_unwind_flag = true;
+
+              ref = SPECPDL_INDEX ();
+              record_unwind_protect_ptr (x_dnd_cleanup_drag_and_drop, f);
+              x_handle_pending_selection_requests ();
+              x_dnd_unwind_flag = false;
+              unbind_to (ref, Qnil);
+            }
+
 #ifdef USE_GTK
           if (xg_pending_quit_event.kind != NO_EVENT)
             {
@@ -15801,6 +15860,15 @@ handle_one_xevent (struct x_display_info *dpyinfo,
         SELECTION_EVENT_DPYINFO (&inev.sie) = dpyinfo;
         SELECTION_EVENT_SELECTION (&inev.sie) = eventp->selection;
         SELECTION_EVENT_TIME (&inev.sie) = eventp->time;
+
+        if ((x_dnd_in_progress
+             && dpyinfo == FRAME_DISPLAY_INFO (x_dnd_frame))
+            || (x_dnd_waiting_for_finish
+                && dpyinfo->display == x_dnd_finish_display))
+          {
+            x_push_selection_request (&inev.sie);
+            EVENT_INIT (inev.ie);
+          }
       }
       break;
 
@@ -15829,17 +15897,7 @@ handle_one_xevent (struct x_display_info *dpyinfo,
             || (x_dnd_waiting_for_finish
                 && dpyinfo->display == x_dnd_finish_display))
           {
-#ifndef USE_GTK
-            eassume (hold_quit);
-#else
-            /* If the debugger runs inside a selection converter, then
-               xg_select can call handle_one_xevent with no
-               hold_quit.  */
-            if (!hold_quit)
-              goto done;
-#endif
-
-            *hold_quit = inev.ie;
+            x_push_selection_request (&inev.sie);
             EVENT_INIT (inev.ie);
           }
 
@@ -21120,12 +21178,16 @@ XTread_socket (struct terminal *terminal, struct input_event *hold_quit)
      read X events while the drag-and-drop event loop is in progress,
      things can go wrong very quick.
 
+     When x_dnd_unwind_flag is true, the above doesn't apply, since
+     the surrounding code takes special precautions to keep it safe.
+
      That doesn't matter for events from displays other than the
      display of the drag-and-drop operation, though.  */
-  if ((x_dnd_in_progress
-       && dpyinfo->display == FRAME_X_DISPLAY (x_dnd_frame))
-      || (x_dnd_waiting_for_finish
-          && dpyinfo->display == x_dnd_finish_display))
+  if (!x_dnd_unwind_flag
+      && ((x_dnd_in_progress
+           && dpyinfo->display == FRAME_X_DISPLAY (x_dnd_frame))
+          || (x_dnd_waiting_for_finish
+              && dpyinfo->display == x_dnd_finish_display)))
     return 0;
 
   block_input ();
@@ -25840,6 +25902,7 @@ x_delete_display (struct x_display_info *dpyinfo)
   struct terminal *t;
   struct color_name_cache_entry *color_entry, *next_color_entry;
   int i;
+  struct x_selection_request_event *ie, *last, *temp;
 
   /* Close all frames and delete the generic struct terminal for this
      X display.  */
@@ -25855,6 +25918,30 @@ x_delete_display (struct x_display_info *dpyinfo)
         break;
       }
 
+  /* Find any pending selection requests for this display and unchain
+     them.  */
+
+  last = NULL;
+
+  for (ie = pending_selection_requests; ie; ie = ie->next)
+    {
+    again:
+
+      if (SELECTION_EVENT_DPYINFO (&ie->se) == dpyinfo)
+        {
+          if (last)
+            last->next = ie->next;
+
+          temp = ie;
+          ie = ie->next;
+          xfree (temp);
+
+          goto again;
+        }
+
+      last = ie;
+    }
+
   if (next_noop_dpyinfo == dpyinfo)
     next_noop_dpyinfo = dpyinfo->next;
 
diff --git a/src/xterm.h b/src/xterm.h
index 878cb5fd87b..22c6b551766 100644
--- a/src/xterm.h
+++ b/src/xterm.h
@@ -1455,6 +1455,8 @@ extern void x_xr_reset_ext_clip (struct frame *f);
 extern void x_scroll_bar_configure (GdkEvent *);
 #endif
 
+extern void x_handle_pending_selection_requests (void);
+extern bool x_detect_pending_selection_requests (void);
 extern Lisp_Object x_dnd_begin_drag_and_drop (struct frame *, Time, Atom,
                                               Lisp_Object, Atom *, const char **,
                                               size_t, bool, Atom *, int);