I have a better understanding of what's going on now, but I still think my
original patch is basically correct. Below
is a simple program that will reliably make vim unresponsive. Just compile/run
the program then do :reg in vim.
#include <stdlib.h>
#include <stdio.h>
#include <unistd.h>
#include <X11/Xlib.h>
#include <X11/Xatom.h>
int
main()
{
Display *const dpy = XOpenDisplay(NULL);
if (!dpy) {
fprintf(stderr, "XOpenDisplay failed!\n");
abort();
}
Window w;
XGetInputFocus(dpy, &w, &(int){0}); // see man
if(w == None) {
fprintf(stderr, "no focus window\n");
XCloseDisplay(dpy);
return 1;
}
// Own clipboards
XSetSelectionOwner(dpy, XA_PRIMARY, w, CurrentTime);
XSetSelectionOwner(dpy, XInternAtom(dpy, "CLIPBOARD", 0), w, CurrentTime);
XFlush(dpy);
// --
while (1);
return 0;
}
> Hmm, strange. I had always thought, and the man page seems to suggest
> this as well, that when XtAppPending() returns non-zero, then
> XtAppNextEvent() will get the next event. The manual is unclear, but
> perhaps an XtIMTimer event is handled inside XtAppPending() and then it
> blocks on waiting for another event?
When a timer goes off, XtAppPending returns with mask XtIMTimer then the event
is handled in XtAppNextEvent.
Unfortunately, XtAppNextEvent won't return until it processes an XtIMXEvent.
Looking at vim _without_ my patch. Consider this gdb session where I started
the above program, put a
breakpoint on the selection callback function and then did :reg in vim. Vim is
locked up after the second callback, so
I sent an abort signal to vim that dropped us back into gdb. From there we can
see that we are still stuck in XtAppNextEvent because there are no XtIMXEvents
to process.
Breakpoint 2, clip_x11_request_selection_cb (w=0xa9d7b0, success=0x87a820
<success>, sel_atom=0x89b930, type=0x7fffffffdb08, value=0x0,
length=0x7fffffffdb00,
format=0x7fffffffdafc) at ui.c:2130
2130 {
(gdb) bt
#0 clip_x11_request_selection_cb (w=0xa9d7b0, success=0x87a820 <success>,
sel_atom=0x89b930, type=0x7fffffffdb08, value=0x0, length=0x7fffffffdb00,
format=0x7fffffffdafc)
at ui.c:2130
#1 0x00007ffff64b9be7 in ?? () from /usr/lib/libXt.so.6
#2 0x00007ffff64b34f6 in ?? () from /usr/lib/libXt.so.6
#3 0x00007ffff64b405f in XtAppNextEvent () from /usr/lib/libXt.so.6
#4 0x0000000000521191 in xterm_update () at os_unix.c:7101
#5 0x00000000005248ec in RealWaitForChar (fd=0, msec=msec@entry=0,
check_for_gpm=check_for_gpm@entry=0x0) at os_unix.c:5469
#6 0x00000000005273e3 in mch_breakcheck () at os_unix.c:5111
#7 0x000000000059cacf in ui_breakcheck () at ui.c:367
#8 0x0000000000511c31 in ex_display (eap=<optimized out>) at ops.c:4195
#9 0x0000000000492b7d in do_one_cmd (cookie=0x0, fgetline=0x4a4a10
<getexline>, cstack=0x7fffffffe1b0, sourcing=0, cmdlinep=0x7fffffffe038) at
ex_docmd.c:2940
#10 do_cmdline (cmdline=cmdline@entry=0x0, fgetline=<optimized out>,
cookie=cookie@entry=0x0, flags=<optimized out>) at ex_docmd.c:1133
#11 0x00000000004fe195 in nv_colon (cap=0x7fffffffe6e0) at normal.c:5373
#12 0x00000000005076ce in normal_cmd (oap=oap@entry=0x7fffffffe7a0,
toplevel=toplevel@entry=1) at normal.c:1160
#13 0x00000000005d63ed in main_loop (cmdwin=0, noexmode=0) at main.c:1334
#14 0x000000000043776f in main (argc=0, argv=0x1) at main.c:1034
(gdb) c
Continuing.
Breakpoint 2, clip_x11_request_selection_cb (w=0xa9d7b0, success=0x87a820
<success>, sel_atom=0xaa6200, type=0x7fffffffdb08, value=0x0,
length=0x7fffffffdb00,
format=0x7fffffffdafc) at ui.c:2130
2130 {
(gdb) bt
#0 clip_x11_request_selection_cb (w=0xa9d7b0, success=0x87a820 <success>,
sel_atom=0xaa6200, type=0x7fffffffdb08, value=0x0, length=0x7fffffffdb00,
format=0x7fffffffdafc)
at ui.c:2130
#1 0x00007ffff64b9be7 in ?? () from /usr/lib/libXt.so.6
#2 0x00007ffff64b34f6 in ?? () from /usr/lib/libXt.so.6
#3 0x00007ffff64b405f in XtAppNextEvent () from /usr/lib/libXt.so.6
#4 0x0000000000521191 in xterm_update () at os_unix.c:7101
#5 0x00000000005248ec in RealWaitForChar (fd=0, msec=msec@entry=0,
check_for_gpm=check_for_gpm@entry=0x0) at os_unix.c:5469
#6 0x00000000005273e3 in mch_breakcheck () at os_unix.c:5111
<snip>
(gdb) c
Continuing.
Program received signal SIGABRT, Aborted.
0x00007ffff489b4d0 in __poll_nocancel () from /usr/lib/libc.so.6
(gdb) bt
#0 0x00007ffff489b4d0 in __poll_nocancel () from /usr/lib/libc.so.6
#1 0x00007ffff64b2c68 in _XtWaitForSomething () from /usr/lib/libXt.so.6
#2 0x00007ffff64b40a7 in XtAppNextEvent () from /usr/lib/libXt.so.6
#3 0x0000000000521191 in xterm_update () at os_unix.c:7101
#4 0x00000000005248ec in RealWaitForChar (fd=0, msec=msec@entry=0,
check_for_gpm=check_for_gpm@entry=0x0) at os_unix.c:5469
#5 0x00000000005273e3 in mch_breakcheck () at os_unix.c:5111
#6 0x000000000059cacf in ui_breakcheck () at ui.c:367
<snip>
> After your change, wouldn't XtIMTimer events be sitting in the queue
> until a character is typed?
Probably not, but the truth is racy. Looking at vim _with_ my patch and again
I'm running the clipboard
hijacking program at the beginning of this email. Frame 2 is the function call
I introduced that will process
a timer event and return.
(gdb) bt
#0 clip_x11_request_selection_cb (w=0xa9d7b0, success=0x87a860 <success>,
sel_atom=0xa78580, type=0x7fffffffda48, value=0x0, length=0x7fffffffda40,
format=0x7fffffffda3c)
at ui.c:2130
#1 0x00007ffff64b9be7 in ?? () from /usr/lib/libXt.so.6
#2 0x00007ffff64b4334 in XtAppProcessEvent () from /usr/lib/libXt.so.6
#3 0x00000000005211e9 in xterm_update () at os_unix.c:7114
#4 0x000000000052496c in RealWaitForChar (fd=0, msec=msec@entry=0,
check_for_gpm=check_for_gpm@entry=0x0) at os_unix.c:5469
#5 0x0000000000527463 in mch_breakcheck () at os_unix.c:5111
#6 0x000000000059cb4f in ui_breakcheck () at ui.c:367
#7 0x0000000000511c81 in ex_display (eap=<optimized out>) at ops.c:4195
#8 0x0000000000492bcd in do_one_cmd (cookie=0x0, fgetline=0x4a4a60
<getexline>, cstack=0x7fffffffe1b0, sourcing=0, cmdlinep=0x7fffffffe038) at
ex_docmd.c:2940
#9 do_cmdline (cmdline=cmdline@entry=0x0, fgetline=<optimized out>,
cookie=cookie@entry=0x0, flags=<optimized out>) at ex_docmd.c:1133
#10 0x00000000004fe1e5 in nv_colon (cap=0x7fffffffe6e0) at normal.c:5373
#11 0x000000000050771e in normal_cmd (oap=oap@entry=0x7fffffffe7a0,
toplevel=toplevel@entry=1) at normal.c:1160
#12 0x00000000005d646d in main_loop (cmdwin=0, noexmode=0) at main.c:1334
#13 0x00000000004377bf in main (argc=0, argv=0x1) at main.c:1034
Looking at line 4195 in ops.c (frame 7), this function call is supposed to be
giving us a chance to catch ctrl+c interrupts. RealWaitForChar calls
xterm_update (where my patch makes it possible to handle the timer event)
before we even call the select() that should let us handle breaks. So in this
sense we are definitely not waiting on a keystroke in order to handle timer
events.
If one were to comment out the ui_breakcheck line in ex_display() you can see
that the multiple dis_msg() calls are also capable of getting us into
xterm_update thus handling the expired timer. It's possible that the timer
won't expire until after we're done with the ex_display(). In this case we
should end up in the high level input loop whose backtrace looks like this.
#0 0x00007ffff489d193 in __select_nocancel () from /usr/lib/libc.so.6
#1 0x00000000005247fb in RealWaitForChar (fd=0, msec=msec@entry=4000,
check_for_gpm=check_for_gpm@entry=0x7fffffffe40c) at os_unix.c:5499
#2 0x0000000000524cff in WaitForChar (msec=<optimized out>) at os_unix.c:5168
#3 0x0000000000524ff4 in mch_inchar (buf=buf@entry=0x8751c4 <typebuf_init+68>
"", maxlen=maxlen@entry=65, wtime=wtime@entry=-1,
tb_change_cnt=tb_change_cnt@entry=38)
at os_unix.c:421
#4 0x000000000059e8d8 in ui_inchar (buf=buf@entry=0x8751c4 <typebuf_init+68>
"", maxlen=maxlen@entry=65, wtime=wtime@entry=-1,
tb_change_cnt=tb_change_cnt@entry=38)
at ui.c:199
#5 0x00000000004b972f in inchar (buf=0x8751c4 <typebuf_init+68> "",
maxlen=196, wait_time=-1, tb_change_cnt=38) at getchar.c:3098
#6 0x00000000004bb715 in vgetorpeek (advance=advance@entry=1) at
getchar.c:2873
#7 0x00000000004bbe8e in vgetc () at getchar.c:1638
#8 0x00000000004bc2e9 in safe_vgetc () at getchar.c:1843
#9 0x0000000000506c1e in normal_cmd (oap=oap@entry=0x7fffffffe7a0,
toplevel=toplevel@entry=1) at normal.c:638
#10 0x00000000005d63ed in main_loop (cmdwin=0, noexmode=0) at main.c:1334
#11 0x000000000043776f in main (argc=0, argv=0x1) at main.c:1034
In this case yes, we're waiting for either a keystroke or for 'msec'
milliseconds to pass (which is the global variable p_ut aka updatetime in
option.c). When we eventually process the callback, this is what it will do.
clip_x11_request_selection_cb(...)
{
// ....
if (*sel_atom == clip_plus.sel_atom)
cbd = &clip_plus;
else
cbd = &clip_star;
if (value == NULL || *length == 0)
{
clip_free_selection(cbd); /* nothing received, clear register */
*(int *)success = FALSE;
return;
}
// ...
}
It looks like there is probably a race condition where someone yanks into */+
(clip_star/clip_plus) then the timer goes off and the callback does
clip_free_selection() which eventually calls free_yank() on the register that
someone just yanked into. So my patch introduces the possibility of data
disappearing from the */+ registers in a racy way.
There is the possibility of a worse outcome if a piece of code does something
like this:
0. stick data in */+ register
1. call xterm_update
2. assume data is still in */+ register, start dereferencing things
This could cause a crash if we process an expired timer @ 1. But such an idiom
should probably be considered incorrect?
###########################################################
###########################################################
At a high level, there are 3 selection owner states.
+ Well behaving owner: vim sends SelectionRequests, owner responds with
SelectionNotify
+ No owner: vim sends SelectionRequests, xserver responds with empty
SelectionNotify
+ Negligent owner: vim sends SelectionRequests, no response is sent
This third case results in vim locking up. We are introducing a race where
data can disappear from a register in order to prevent vim from locking up when
the selection owner is misbehaving.
There is of course the possibility of introducing the fix in some other way,
but I don't currently see a simplish way to do so.
--
Aaron Burrow
--
--
You received this message from the "vim_dev" maillist.
Do not top-post! Type your reply below the text you are replying to.
For more information, visit http://www.vim.org/maillist.php
---
You received this message because you are subscribed to the Google Groups
"vim_dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/d/optout.
