On 07/10/2012 06:31 AM, Bram Moolenaar wrote:
>
> Brian Burns wrote:
>
>> I've attached some basic tests.
>> I'm not sure if what I'm doing here would work on all systems,
>> but it's a start. Let me know what you think.
>
> Thanks. It's nice to have a test for this, there was nothing before.
>
> It is rather Unix-specific, but that's hard to avoid. We can only run
> this test from the Unix Makefile so that we don't even try on other
> systems.
>
> Even the Unix version may not have the clientserver feature. We can
> check that at the start:
>
> STARTTEST
> :so small.vim
> :" drop out when the clientserver features is not supported
> :if !has("clientserver")
> : e! test.ok
> : w! test.out
> : qa!
> :endif
>
> If the test fails halfway then the test server keeps running. I think
> it's better to kill it from the Makefile.
>
> You can use v:progname for the Vim program name. It doesn't include the
> path though.
>
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().
In running the test in a loop, I discovered I was getting intermittent
failures for remote_peek(). By placing a forced call to serverEventProc
in the top of this method, I could run the tests 1000's of times with no
failures. So, I added some logging in various places to track down why
this XEvent was missing, even though the commProperty change was taking
place. There is apparently some timing issue with X Toolkit. If Vim is
started and receives a property change quickly, the call to XtAppPending
fails to report the PropertyChange Event. The event is there, as I've
been able to confirm that a call to XCheckWindowEvent will always see it
every time XtAppPending does not. The other good news is that when I ran
the test so that it performed 10 remote_send->server2client->remote_peek
calls, only the first call would ever fail. If the first call failed,
then all 9 following calls always succeeded. In fact, if I placed a
:sleep call in the top of the test file, I could reduce the number of
failures. The longer the delay, the fewer failures. However, even at
500m, I was able to get a failure in the first few hundred tests.
I haven't looked into the sources for XtAppPending to see why this is
happening, but using a call to XCheckWindowEvent certainly seems to
solve the problem.
This of course affects remote_read() as well, since it uses ServerWait
with localLoop set FALSE. However, now that ServerWait is calling
XCheckWindowEvent in either case, this is picking up any event missed by
XtAppPending. I also tried a remote_expr() using gvim (GTK), and it
showed the same problem. However, I was not able to get gvim to fail
using the remote_send->server2client->remote_peek call. I guess the
server's turn around time for this is slightly more than responding to
an expr request. In any case, the XCheckWindowEvent call solves it.
The other problem is that this means that incoming commands to a Vim
server could be missed if they're sent to quickly after the server is
started. Without knowing exactly what the problem/delay is with
XtAppPending, it would be hard to know at what point after the server
started you would need to issue an XCheckWindowEvent call to work around
this - if you even wanted to do this. Then again, I don't see this as
being an issue as much as clients receiving responses. It may be enough
to simply note that if you start a server, not to send it any commands
immediately. With the current test, there's a 0.2 second delay after
server startup, plus whatever the delay is before the test actually
starts, and I haven't seen any failure where the server misses the first
command. But since I was able to get a client to fail with a 500 msec
delay, I'm not sure what the recommendation would be.
So, other than the server startup issue, I think these changes to
ServerWait and serverPeekReply should take care of this missing event
issue. I may try to track that down in the X Toolkit, since I've gotten
curious :) Also, I think I'll look into adding to this test some calls
using the command line options (--remote-*). I tried the following in
the Makefile and it works:
vartest:
VAR="$(VIMPROG)" $(VIMPROG) -u NONE \
+'echomsg system("echo $$VAR") | sleep 1 | q'
So, I should be able to grab this in the top of the test and use it to
make system() calls.
Something else I noticed earler but hadn't mentioned...
In serverEventProc, the call to XGetWindowProperty says it will return
bytesAfter should a partial read be performed. MAX_PROP_WORDS is setting
the size of the data to be read to 400kB. It's unclear to me if the
function is guaranteed to return this amount. Does 'partial read' mean
there's more than this amount in the property, or that the function is
for some reason returning only a portion of what you've requested?
In any case, I don't see anything limiting the amount of data being
stored in the property. And it will apparently hold as much as the
X Server and/or memory will allow.
--
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 11d40fc82f11 src/if_xcmdsrv.c
--- a/src/if_xcmdsrv.c Thu Jul 12 22:01:11 2012 +0200
+++ b/src/if_xcmdsrv.c Fri Jul 13 08:04:39 2012 -0400
@@ -594,6 +594,12 @@
time(&start);
while (TRUE)
{
+ /* This is being used when localLoop is FALSE in order to catch
+ * XEvents which the X Toolkit will intermittently fail to report.
+ * This will happen when Vim is launched and very quickly receives
+ * a change to it's commProperty.
+ * e.g. (g)vim -u NONE +'echomsg remote_expr("server_name", "2+2")'
+ */
while (XCheckWindowEvent(dpy, commWindow, PropertyChangeMask, &event))
serverEventProc(dpy, &event);
@@ -850,6 +856,21 @@
{
struct ServerReply *p;
+ /* This is being used in order to catch XEvents which the X Toolkit
+ * will intermittently fail to report. This will happen when Vim is
+ * launched and very quickly receives a change to it's commProperty.
+ * 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 11d40fc82f11 src/testdir/Makefile
--- a/src/testdir/Makefile Thu Jul 12 22:01:11 2012 +0200
+++ b/src/testdir/Makefile Fri Jul 13 08:04:39 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
SCRIPTS_GUI = test16.out
@@ -79,5 +79,12 @@
test60.out: test60.vim
+test88: test88.server test88.out
+ -/bin/sh -c "pkill -f vim_server_test88"
+
+test88.server:
+ -$(VIMPROG) -nes -u NONE --servername vim_server_test88 <&- 2>&1
>/dev/null &
+ @-sleep .2
+
nolog:
-rm -f test.log
diff -r 11d40fc82f11 src/testdir/test88.in
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/src/testdir/test88.in Fri Jul 13 08:04:39 2012 -0400
@@ -0,0 +1,95 @@
+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
+:"
+:" Store results and exit
+:"
+G"ap
+:?RESULTS?1,$w! test.out
+:qa!
+ENDTEST
+
+RESULTS
diff -r 11d40fc82f11 src/testdir/test88.ok
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/src/testdir/test88.ok Fri Jul 13 08:04:39 2012 -0400
@@ -0,0 +1,1 @@
+ok
