On 07/14/2012 06:21 AM, Bram Moolenaar wrote:
>
>> Brian Burns wrote:
>>
>>
>> I've attached a patch which includes an updated test and Makefile, with
>> the server being started and stopped in the Makefile. Also included is a
>> change to serverPeekReply() and an added note for ServerWait().
>
> [...]
>
> Thanks! I'll look into it later.
>
I've attached a new patch, with an updated test and Makefile which
includes tests using --remote-send and --remote-expr.
Also, I've spent some time with gdb looking into this "missing event"
issue and I've determined what the root cause is.
When ServerWait is called with localLoop FALSE, ui_delay is used and
ultimately calls RealWaitForChar. This function is using select/poll to
determine if there is an event waiting in order to call xterm_update.
However, this is not an accurate test, since the X server reads that fd.
In this case, it's doing so before RealWaitForChar has a chance to poll
the fd. So, even if select/poll returns 0 or FD_ISSET() returns false,
events may already be in the X queue, and a call to XtAppPending would
find it. In light of this, I was thinking that RealWaitForChar needs to
calls xterm_update regardless. Something like:
if (ret >= 0 && xterm_Shell != (Widget)0)
{
xterm_update(); /* Maybe we should hand out clipboard */
/* continue looping when we only got the X event and the input
* buffer is empty */
if (((ret == 1 && FD_ISSET(ConnectionNumber(xterm_dpy), &rfds))
|| ret == 0) && !input_available())
{
/* Try again */
finished = FALSE;
}
}
While this appears to work in the debugger (so far), running the test
actually hangs, as something is waiting indefinitely for input.
Sometimes hitting return will allow the test to finish, sometimes you
have break out and forcibly kill the server. I've tried a few variations
on this, but at this point, I haven't tracked this down.
The fact that checking the fd isn't a true test of waiting events would
be the same in ServerWait when localLoop is TRUE. However, in this case
that loop is entered quickly; before the server has a chance to respond.
So you have a much better chance at detecting it's response. And, since
this loop checks the queue each time select/poll times out anyway, it's
not an issue.
For serverPeekReply, unless RealWaitForChar can call xterm_update or
check the X queue somehow, this function still needs to do this itself.
In fact, it may be best that it does this regardless.
In the end, it may be best to leave things as they are - with the
addition of the queue check in serverPeekReply. This, along with
checking the queue in ServerWait even when localLoop is FALSE, seems to
handle this issue. But I wanted to point this out, as I'm not sure what
other events (clipboard?) RealWaitForChar may miss due to this.
Also, I had mentioned that using gvim (GTK) with the following would
occasionally hang, yet I just ran this a few times with no failures:
for i in {1..100}; do
./gvim -u NONE +'echomsg remote_expr("vim_server_test", "2+2") | q';
done
Given that it's compiling as 'vim', I may have forgotten to copy this to
'gvim' or run it against my system gvim (7.3.515-2.fc16.i686) or
something. I stepped through this with gdb as well and didn't see any
problems. Gotta love intermittent errors :)
One more thing... I was able to run the tests in this patch (and all the
tests) against 'gvim' by copying src/vim to src/gvim, then changing
VIMPROG to ../gvim and adding '-f' to the command lines in the Makefile.
Is this how you would do this, or am I missing something?
--
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
diff -r 942df3a8051b src/if_xcmdsrv.c
--- a/src/if_xcmdsrv.c Mon Jul 16 19:27:29 2012 +0200
+++ b/src/if_xcmdsrv.c Tue Jul 17 06:37:02 2012 -0400
@@ -594,6 +594,10 @@
time(&start);
while (TRUE)
{
+ /* This is being used when localLoop is FALSE in order to catch
+ * XEvents which may be missed by the main ui loop.
+ * e.g. vim -u NONE +'echomsg remote_expr("server_name", "2+2")'
+ */
while (XCheckWindowEvent(dpy, commWindow, PropertyChangeMask, &event))
serverEventProc(dpy, &event);
@@ -850,6 +854,20 @@
{
struct ServerReply *p;
+ /* Check the XEvent queue for any new server replies.
+ * Also used to catch XEvents which may be missed by the main ui loop.
+ * Without this, the following would produce some "0" responses:
+ *
+ * for i in {1..10};
+ * do vim -u NONE +"call remote_send('server_name', \
+ * \":call server2client(expand('<client>'), '')<CR>\", 'serverid') \
+ * | sleep 1 | echomsg remote_peek(serverid) | sleep 200m | q";
+ * done
+ */
+ XEvent event;
+ while (XCheckWindowEvent(dpy, commWindow, PropertyChangeMask, &event))
+ serverEventProc(dpy, &event);
+
if ((p = ServerReplyFind(win, SROP_Find)) != NULL && p->strings.ga_len > 0)
{
if (str != NULL)
diff -r 942df3a8051b src/testdir/Makefile
--- a/src/testdir/Makefile Mon Jul 16 19:27:29 2012 +0200
+++ b/src/testdir/Makefile Tue Jul 17 06:37:02 2012 -0400
@@ -27,7 +27,7 @@
test69.out test70.out test71.out test72.out test73.out \
test74.out test75.out test76.out test77.out test78.out \
test79.out test80.out test81.out test82.out test83.out \
- test84.out test85.out test86.out test87.out
+ test84.out test85.out test86.out test87.out test88.out
SCRIPTS_GUI = test16.out
@@ -79,5 +79,22 @@
test60.out: test60.vim
+test88.out:
+ @-$(VIMPROG) -nes -u NONE --servername vim_server_test88 <&- 2>&1
>/dev/null &
+ @-sleep .2
+ -rm -rf $*.failed test.ok test.out X* viminfo
+ cp $*.ok test.ok
+ # Sleep a moment to avoid that the xterm title is messed up
+ @-sleep .2
+ -VIMEXEC="$(VIMPROG)" $(VALGRIND) $(VIMPROG) -u unix.vim -U NONE
--noplugin -s dotest.in $*.in
+ @/bin/sh -c "if test -f test.out; then\
+ if diff test.out $*.ok; \
+ then mv -f test.out $*.out; \
+ else echo $* FAILED >>test.log; mv -f test.out $*.failed; \
+ fi \
+ else echo $* NO OUTPUT >>test.log; \
+ fi"
+ @-/bin/sh -c "pkill -f vim_server_test88"
+
nolog:
-rm -f test.log
diff -r 942df3a8051b src/testdir/test88.in
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/src/testdir/test88.in Tue Jul 17 06:37:02 2012 -0400
@@ -0,0 +1,129 @@
+Tests for the +clientserver feature.
+
+The 'vim_server_test88' server is started and stopped within the Makefile.
+
+STARTTEST
+:so small.vim
+:" drop out when the clientserver features is not supported
+:if !has("clientserver")
+: e! test.ok
+: w! test.out
+: qa!
+:endif
+:"
+:" Set register 'a' to our expected result of 'ok'.
+:" Any error messages will be appended.
+:"
+:call setreg('a', 'ok', 'l')
+:"
+:" Verify the server is available.
+:"
+:if index(split(serverlist(), "\n"), 'VIM_SERVER_TEST88') < 0
+: if empty(system("../vim --serverlist | grep -i vim_server_test88"))
+: call setreg('a', 'Server failed to start.', 'al')
+: else
+: call setreg('a', 'serverlist() failed.', 'al')
+: endif
+:endif
+:"
+:" Make sure remote_expr() works.
+:"
+:let reply = remote_expr('vim_server_test88', '2+2')
+:if reply != "4"
+: call setreg('a', 'remote_expr() failed.', 'al')
+: call setreg('a', 'Expected: ''4'' Got: '. reply, 'al')
+:endif
+:"
+:" Set a line in server's buffer.
+:" Give this a chance to process, since remote_send() returns immediately.
+:" Ask the server for the line to verify it was set.
+:"
+:call remote_send('vim_server_test88', 'call setline(1,"test")<cr>')
+:sleep 200m
+:let reply = remote_expr('vim_server_test88', 'getline(1)')
+:if reply != 'test'
+: call setreg('a', 'remote_send() failed to set server''s buffer.', 'al')
+: call setreg('a', 'Expected: ''test'' Got: '. reply, 'al')
+:endif
+:"
+:" Lists should be returned as a newline joined string.
+:"
+:call remote_expr('vim_server_test88', 'setline(1,["one","two"])')
+:let reply = remote_expr('vim_server_test88', 'getline(1,"$")')
+:if reply != "one\ntwo\n"
+: call setreg('a', 'remote_expr() failed to return a newline joined string.',
'al')
+: call setreg('a', 'Expected: ''one\ntwo\n'' Got: '. reply, 'al')
+:endif
+:"
+:" Ask the server to send us a message.
+:" Verify the message is waiting.
+:" Read the message.
+:"
+:let sid = ''
+:call remote_send('vim_server_test88', 'call server2client(expand("<client>"),
"message")<cr>', 'sid')
+:if empty(sid)
+: call setreg('a', 'remote_send() failed to set the server id.', 'al')
+:else
+: let reply = ''
+: for i in range(10)
+: sleep 100m
+: let avail = remote_peek(sid, 'reply')
+: if avail > 0 | break | endif
+: endfor
+: if avail < 1
+: call setreg('a', 'remote_peek() failed to report a message is waiting.',
'al')
+: else
+: if reply != 'message'
+: call setreg('a', 'remote_peek() failed to return the waiting message.',
'al')
+: call setreg('a', 'Expected: ''message'' Got: '. reply, 'al')
+: endif
+: let reply = remote_read(sid)
+: if reply != 'message'
+: call setreg('a', 'remote_read() failed to return the waiting message.',
'al')
+: call setreg('a', 'Expected: ''message'' Got: '. reply, 'al')
+: endif
+: endif
+:endif
+:"
+:" Get the path to VIMPROG from the Makefile (pass as an ENV variable)
+:" and append the servername option for testing --remote-send/--remote-expr
+:"
+:let vimexec = $VIMEXEC. ' --servername vim_server_test88 '
+:"
+:" Make sure --remote-expr works.
+:"
+:let reply = system(vimexec. '--remote-expr "2+2"')
+:if reply != "4\n"
+: call setreg('a', '--remote-expr failed.', 'al')
+: call setreg('a', 'Expected: ''4\n'' Got: '. reply, 'al')
+:endif
+:"
+:" Set a line in server's buffer.
+:" Give this a chance to process, since --remote-send returns immediately.
+:" Ask the server for the line to verify it was set.
+:"
+:call system(vimexec. '--remote-send ":call setline(1,\"test\")<cr>"')
+:sleep 200m
+:let reply = system(vimexec. '--remote-expr "getline(1)"')
+:if reply != "test\n"
+: call setreg('a', '--remote-send failed to set server''s buffer.', 'al')
+: call setreg('a', 'Expected: ''test\n'' Got: '. reply, 'al')
+:endif
+:"
+:" Lists should be returned as a newline joined string.
+:"
+:call system(vimexec. '--remote-expr "setline(1,[\"one\",\"two\"])"')
+:let reply = system(vimexec. '--remote-expr "getline(1,\"$\")"')
+:if reply != "one\ntwo\n"
+: call setreg('a', '--remote-expr failed to return a newline joined string.',
'al')
+: call setreg('a', 'Expected: ''one\ntwo\n'' Got: '. reply, 'al')
+:endif
+:"
+:" Store results and exit
+:"
+G"ap
+:?RESULTS?1,$w! test.out
+:qa!
+ENDTEST
+
+RESULTS
diff -r 942df3a8051b src/testdir/test88.ok
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/src/testdir/test88.ok Tue Jul 17 06:37:02 2012 -0400
@@ -0,0 +1,1 @@
+ok
