]> git.ipfire.org Git - thirdparty/vim.git/commitdiff
patch 8.0.0492: a failing client-server request can make Vim hang v8.0.0492
authorBram Moolenaar <Bram@vim.org>
Sun, 19 Mar 2017 20:20:53 +0000 (21:20 +0100)
committerBram Moolenaar <Bram@vim.org>
Sun, 19 Mar 2017 20:20:53 +0000 (21:20 +0100)
Problem:    A failing client-server request can make Vim hang.
Solution:   Add a timeout argument to functions that wait.

runtime/doc/eval.txt
src/evalfunc.c
src/if_xcmdsrv.c
src/main.c
src/os_mswin.c
src/proto/if_xcmdsrv.pro
src/proto/os_mswin.pro
src/testdir/test_clientserver.vim
src/version.c
src/vim.h

index 0387e32461f904d4fc19a10a684a857986de5843..5fa31a2e73f16cf353d4ede451d30e6a9771a017 100644 (file)
@@ -6320,15 +6320,17 @@ reltimestr({time})                              *reltimestr()*
                {only available when compiled with the |+reltime| feature}
 
                                                        *remote_expr()* *E449*
-remote_expr({server}, {string} [, {idvar}])
+remote_expr({server}, {string} [, {idvar} [, {timeout}]])
                Send the {string} to {server}.  The string is sent as an
                expression and the result is returned after evaluation.
                The result must be a String or a |List|.  A |List| is turned
                into a String by joining the items with a line break in
                between (not at the end), like with join(expr, "\n").
-               If {idvar} is present, it is taken as the name of a
-               variable and a {serverid} for later use with
+               If {idvar} is present and not empty, it is taken as the name
+               of a variable and a {serverid} for later use with
                remote_read() is stored there.
+               If {timeout} is given the read times out after this many
+               seconds.  Otherwise a timeout of 600 seconds is used.
                See also |clientserver| |RemoteReply|.
                This function is not available in the |sandbox|.
                {only available when compiled with the |+clientserver| feature}
@@ -6367,9 +6369,10 @@ remote_peek({serverid} [, {retvar}])             *remote_peek()*
                        :let repl = ""
                        :echo "PEEK: ".remote_peek(id, "repl").": ".repl
 
-remote_read({serverid})                                *remote_read()*
+remote_read({serverid}, [{timeout}])                   *remote_read()*
                Return the oldest available reply from {serverid} and consume
-               it.  It blocks until a reply is available.
+               it.  Unless a {timeout} in seconds is given, it blocks until a
+               reply is available.
                See also |clientserver|.
                This function is not available in the |sandbox|.
                {only available when compiled with the |+clientserver| feature}
index a00c75313af815ef464df905fe96eca085707dad..83ff48f8cabf4e09266d0e14fe851ebfc82dbf14 100644 (file)
@@ -739,10 +739,10 @@ static struct fst
     {"reltimefloat",   1, 1, f_reltimefloat},
 #endif
     {"reltimestr",     1, 1, f_reltimestr},
-    {"remote_expr",    2, 3, f_remote_expr},
+    {"remote_expr",    2, 4, f_remote_expr},
     {"remote_foreground", 1, 1, f_remote_foreground},
     {"remote_peek",    1, 2, f_remote_peek},
-    {"remote_read",    1, 1, f_remote_read},
+    {"remote_read",    1, 2, f_remote_read},
     {"remote_send",    2, 3, f_remote_send},
     {"remote_startserver", 1, 1, f_remote_startserver},
     {"remove",         2, 3, f_remove},
@@ -8515,6 +8515,7 @@ remote_common(typval_T *argvars, typval_T *rettv, int expr)
     char_u     *keys;
     char_u     *r = NULL;
     char_u     buf[NUMBUFLEN];
+    int                timeout = 0;
 # ifdef WIN32
     HWND       w;
 # else
@@ -8528,16 +8529,19 @@ remote_common(typval_T *argvars, typval_T *rettv, int expr)
     if (check_connection() == FAIL)
        return;
 # endif
+    if (argvars[2].v_type != VAR_UNKNOWN
+           && argvars[3].v_type != VAR_UNKNOWN)
+       timeout = get_tv_number(&argvars[3]);
 
     server_name = get_tv_string_chk(&argvars[0]);
     if (server_name == NULL)
        return;         /* type error; errmsg already given */
     keys = get_tv_string_buf(&argvars[1], buf);
 # ifdef WIN32
-    if (serverSendToVim(server_name, keys, &r, &w, expr, TRUE) < 0)
+    if (serverSendToVim(server_name, keys, &r, &w, expr, timeout, TRUE) < 0)
 # else
-    if (serverSendToVim(X_DISPLAY, server_name, keys, &r, &w, expr, 0, TRUE)
-                                                                         < 0)
+    if (serverSendToVim(X_DISPLAY, server_name, keys, &r, &w, expr, timeout,
+                                                                 0, TRUE) < 0)
 # endif
     {
        if (r != NULL)
@@ -8555,13 +8559,15 @@ remote_common(typval_T *argvars, typval_T *rettv, int expr)
        char_u          str[30];
        char_u          *idvar;
 
-       sprintf((char *)str, PRINTF_HEX_LONG_U, (long_u)w);
-       v.di_tv.v_type = VAR_STRING;
-       v.di_tv.vval.v_string = vim_strsave(str);
        idvar = get_tv_string_chk(&argvars[2]);
-       if (idvar != NULL)
+       if (idvar != NULL && *idvar != NUL)
+       {
+           sprintf((char *)str, PRINTF_HEX_LONG_U, (long_u)w);
+           v.di_tv.v_type = VAR_STRING;
+           v.di_tv.vval.v_string = vim_strsave(str);
            set_var(idvar, &v.di_tv, FALSE);
-       vim_free(v.di_tv.vval.v_string);
+           vim_free(v.di_tv.vval.v_string);
+       }
     }
 }
 #endif
@@ -8633,7 +8639,7 @@ f_remote_peek(typval_T *argvars UNUSED, typval_T *rettv)
        rettv->vval.v_number = -1;
     else
     {
-       s = serverGetReply((HWND)n, FALSE, FALSE, FALSE);
+       s = serverGetReply((HWND)n, FALSE, FALSE, FALSE, 0);
        rettv->vval.v_number = (s != NULL);
     }
 # else
@@ -8670,17 +8676,23 @@ f_remote_read(typval_T *argvars UNUSED, typval_T *rettv)
 
     if (serverid != NULL && !check_restricted() && !check_secure())
     {
+       int timeout = 0;
+
+       if (argvars[1].v_type != VAR_UNKNOWN)
+           timeout = get_tv_number(&argvars[1]);
+
 # ifdef WIN32
        /* The server's HWND is encoded in the 'id' parameter */
        long_u          n = 0;
 
        sscanf((char *)serverid, SCANF_HEX_LONG_U, &n);
        if (n != 0)
-           r = serverGetReply((HWND)n, FALSE, TRUE, TRUE);
+           r = serverGetReply((HWND)n, FALSE, TRUE, TRUE, timeout);
        if (r == NULL)
 # else
-       if (check_connection() == FAIL || serverReadReply(X_DISPLAY,
-               serverStrToWin(serverid), &r, FALSE) < 0)
+       if (check_connection() == FAIL
+               || serverReadReply(X_DISPLAY, serverStrToWin(serverid),
+                                                      &r, FALSE, timeout) < 0)
 # endif
            EMSG(_("E277: Unable to read a server reply"));
     }
index 9ff6d76056c51bb6f92a7dac92f72618f2ed1a74..4c3c0122024fc305ea8c714935800b68f738bc82 100644 (file)
@@ -373,6 +373,7 @@ serverSendToVim(
     char_u     **result,               /* Result of eval'ed expression */
     Window     *server,                /* Actual ID of receiving app */
     Bool       asExpr,                 /* Interpret as keystrokes or expr ? */
+    int                timeout,                /* seconds to wait or zero */
     Bool       localLoop,              /* Throw away everything but result */
     int                silent)                 /* don't complain about no server */
 {
@@ -485,7 +486,8 @@ serverSendToVim(
     pending.nextPtr = pendingCommands;
     pendingCommands = &pending;
 
-    ServerWait(dpy, w, WaitForPend, &pending, localLoop, 600);
+    ServerWait(dpy, w, WaitForPend, &pending, localLoop,
+                                                 timeout > 0 ? timeout : 600);
 
     /*
      * Unregister the information about the pending command
@@ -790,6 +792,7 @@ WaitForReply(void *p)
 
 /*
  * Wait for replies from id (win)
+ * When "timeout" is non-zero wait up to this many seconds.
  * Return 0 and the malloc'ed string when a reply is available.
  * Return -1 if the window becomes invalid while waiting.
  */
@@ -798,13 +801,15 @@ serverReadReply(
     Display    *dpy,
     Window     win,
     char_u     **str,
-    int                localLoop)
+    int                localLoop,
+    int                timeout)
 {
     int                len;
     char_u     *s;
     struct     ServerReply *p;
 
-    ServerWait(dpy, win, WaitForReply, &win, localLoop, -1);
+    ServerWait(dpy, win, WaitForReply, &win, localLoop,
+                                                  timeout > 0 ? timeout : -1);
 
     if ((p = ServerReplyFind(win, SROP_Find)) != NULL && p->strings.ga_len > 0)
     {
index 52370dc4193a9655583b33eec1bdb53939a589ba..66db3ebc7b1d53b205dbfc0988d194fd7e8b4fe5 100644 (file)
@@ -3791,10 +3791,10 @@ cmdsrv_main(
            }
            else
                ret = serverSendToVim(xterm_dpy, sname, *serverStr,
-                                                   NULL, &srv, 0, 0, silent);
+                                                 NULL, &srv, 0, 0, 0, silent);
 # else
            /* Win32 always works? */
-           ret = serverSendToVim(sname, *serverStr, NULL, &srv, 0, silent);
+           ret = serverSendToVim(sname, *serverStr, NULL, &srv, 0, 0, silent);
 # endif
            if (ret < 0)
            {
@@ -3854,11 +3854,11 @@ cmdsrv_main(
                while (memchr(done, 0, numFiles) != NULL)
                {
 # ifdef WIN32
-                   p = serverGetReply(srv, NULL, TRUE, TRUE);
+                   p = serverGetReply(srv, NULL, TRUE, TRUE, 0);
                    if (p == NULL)
                        break;
 # else
-                   if (serverReadReply(xterm_dpy, srv, &p, TRUE) < 0)
+                   if (serverReadReply(xterm_dpy, srv, &p, TRUE, -1) < 0)
                        break;
 # endif
                    j = atoi((char *)p);
@@ -3885,12 +3885,12 @@ cmdsrv_main(
 # ifdef WIN32
            /* Win32 always works? */
            if (serverSendToVim(sname, (char_u *)argv[i + 1],
-                                                   &res, NULL, 1, FALSE) < 0)
+                                                 &res, NULL, 1, 0, FALSE) < 0)
 # else
            if (xterm_dpy == NULL)
                mch_errmsg(_("No display: Send expression failed.\n"));
            else if (serverSendToVim(xterm_dpy, sname, (char_u *)argv[i + 1],
-                                                &res, NULL, 1, 1, FALSE) < 0)
+                                              &res, NULL, 1, 0, 1, FALSE) < 0)
 # endif
            {
                if (res != NULL && *res != NUL)
index bddb08d920b93c4519aba427487b57acc63a6740..cf8adda7641496a06c0f759486038d24e9037262 100644 (file)
@@ -2401,6 +2401,7 @@ serverSendToVim(
     char_u      **result,              /* Result of eval'ed expression */
     void        *ptarget,              /* HWND of server */
     int                 asExpr,                /* Expression or keys? */
+    int                 timeout,               /* timeout in seconds or zero */
     int                 silent)                /* don't complain about no server */
 {
     HWND       target;
@@ -2444,7 +2445,7 @@ serverSendToVim(
        return -1;
 
     if (asExpr)
-       retval = serverGetReply(target, &retcode, TRUE, TRUE);
+       retval = serverGetReply(target, &retcode, TRUE, TRUE, timeout);
 
     if (result == NULL)
        vim_free(retval);
@@ -2521,14 +2522,17 @@ save_reply(HWND server, char_u *reply, int expr)
  * if "wait" is TRUE block until a message arrives (or the server exits).
  */
     char_u *
-serverGetReply(HWND server, int *expr_res, int remove, int wait)
+serverGetReply(HWND server, int *expr_res, int remove, int wait, int timeout)
 {
     int                i;
     char_u     *reply;
     reply_T    *rep;
     int                did_process = FALSE;
+    time_t     start;
+    time_t     now;
 
     /* When waiting, loop until the message waiting for is received. */
+    time(&start);
     for (;;)
     {
        /* Reset this here, in case a message arrives while we are going
@@ -2584,6 +2588,10 @@ serverGetReply(HWND server, int *expr_res, int remove, int wait)
 #ifdef FEAT_TIMERS
            check_due_timer();
 #endif
+           time(&now);
+           if (timeout > 0 && (now - start) >= timeout)
+               break;
+
            /* Wait for a SendMessage() call to us.  This could be the reply
             * we are waiting for.  Use a timeout of a second, to catch the
             * situation that the server died unexpectedly. */
index 137020e13e3b6d9dc490b8cb4914b9ea1338d8a3..801dc00e0c7c3ef9c878b6d516995358b97e83c5 100644 (file)
@@ -1,11 +1,11 @@
 /* if_xcmdsrv.c */
 int serverRegisterName(Display *dpy, char_u *name);
 void serverChangeRegisteredWindow(Display *dpy, Window newwin);
-int serverSendToVim(Display *dpy, char_u *name, char_u *cmd, char_u **result, Window *server, int asExpr, int localLoop, int silent);
+int serverSendToVim(Display *dpy, char_u *name, char_u *cmd, char_u **result, Window *server, int asExpr, int timeout, int localLoop, int silent);
 char_u *serverGetVimNames(Display *dpy);
 Window serverStrToWin(char_u *str);
 int serverSendReply(char_u *name, char_u *str);
-int serverReadReply(Display *dpy, Window win, char_u **str, int localLoop);
+int serverReadReply(Display *dpy, Window win, char_u **str, int localLoop, int timeout);
 int serverPeekReply(Display *dpy, Window win, char_u **str);
 void serverEventProc(Display *dpy, XEvent *eventPtr, int immediate);
 void server_parse_messages(void);
index 8cfcb439cc13c1ec9ab077eb35befddfb37b68be..cc660a6d9719be1f10b3e7402fca3de7c73fcaba 100644 (file)
@@ -43,9 +43,9 @@ void serverInitMessaging(void);
 void serverSetName(char_u *name);
 char_u *serverGetVimNames(void);
 int serverSendReply(char_u *name, char_u *reply);
-int serverSendToVim(char_u *name, char_u *cmd, char_u **result, void *ptarget, int asExpr, int silent);
+int serverSendToVim(char_u *name, char_u *cmd, char_u **result, void *ptarget, int asExpr, int timeout, int silent);
 void serverForeground(char_u *name);
-char_u *serverGetReply(HWND server, int *expr_res, int remove, int wait);
+char_u *serverGetReply(HWND server, int *expr_res, int remove, int wait, int timeout);
 void serverProcessPendingMessages(void);
 char *charset_id2name(int id);
 char *quality_id2name(DWORD id);
index d97f5ea20e0d71a5fa0c87866422ccf59cd4da1d..c98fc026ae1e0e21ec2ffbe88f67ecc768600be0 100644 (file)
@@ -6,22 +6,12 @@ endif
 
 source shared.vim
 
-let s:where = 0
-func Abort(id)
-  call assert_report('Test timed out at ' . s:where)
-  call FinishTesting()
-endfunc
-
 func Test_client_server()
   let cmd = GetVimCommand()
   if cmd == ''
     return
   endif
 
-  " Some of these commands may hang when failing.
-  call timer_start(10000, 'Abort')
-
-  let s:where = 1
   let name = 'XVIMTEST'
   let cmd .= ' --servername ' . name
   let g:job = job_start(cmd, {'stoponexit': 'kill', 'out_io': 'null'})
@@ -30,62 +20,53 @@ func Test_client_server()
     call assert_report('Cannot run the Vim server')
     return
   endif
-  let s:where = 2
 
   " Takes a short while for the server to be active.
   call WaitFor('serverlist() =~ "' . name . '"')
   call assert_match(name, serverlist())
-  let s:where = 3
 
   call remote_foreground(name)
-  let s:where = 4
 
   call remote_send(name, ":let testvar = 'yes'\<CR>")
-  let s:where = 5
-  call WaitFor('remote_expr("' . name . '", "testvar") == "yes"')
-  let s:where = 6
-  call assert_equal('yes', remote_expr(name, "testvar"))
-  let s:where = 7
+  call WaitFor('remote_expr("' . name . '", "testvar", "", 1) == "yes"')
+  call assert_equal('yes', remote_expr(name, "testvar", "", 2))
 
   if has('unix') && has('gui') && !has('gui_running')
     " Running in a terminal and the GUI is avaiable: Tell the server to open
     " the GUI and check that the remote command still works.
     " Need to wait for the GUI to start up, otherwise the send hangs in trying
     " to send to the terminal window.
-    call remote_send(name, ":gui -f\<CR>")
-    let s:where = 8
-    sleep 500m
+    if has('gui_athena') || has('gui_motif')
+      " For those GUIs, ignore the 'failed to create input context' error.
+      call remote_send(name, ":call test_ignore_error('E285') | gui -f\<CR>")
+    else
+      call remote_send(name, ":gui -f\<CR>")
+    endif
+    " Wait for the server to be up and answering requests.
+    call WaitFor('remote_expr("' . name . '", "v:version", "", 1) != ""')
+
     call remote_send(name, ":let testvar = 'maybe'\<CR>")
-    let s:where = 9
-    call WaitFor('remote_expr("' . name . '", "testvar") == "maybe"')
-    let s:where = 10
-    call assert_equal('maybe', remote_expr(name, "testvar"))
-    let s:where = 11
+    call WaitFor('remote_expr("' . name . '", "testvar", "", 1) == "maybe"')
+    call assert_equal('maybe', remote_expr(name, "testvar", "", 2))
   endif
 
   call assert_fails('call remote_send("XXX", ":let testvar = ''yes''\<CR>")', 'E241')
-  let s:where = 12
 
   " Expression evaluated locally.
   if v:servername == ''
     call remote_startserver('MYSELF')
-    let s:where = 13
-    call assert_equal('MYSELF', v:servername)
+    " May get MYSELF1 when running the test again.
+    call assert_match('MYSELF', v:servername)
   endif
   let g:testvar = 'myself'
   call assert_equal('myself', remote_expr(v:servername, 'testvar'))
-  let s:where = 14
 
   call remote_send(name, ":call server2client(expand('<client>'), 'got it')\<CR>", 'g:myserverid')
-  let s:where = 15
-  call assert_equal('got it', remote_read(g:myserverid))
-  let s:where = 16
+  call assert_equal('got it', remote_read(g:myserverid, 2))
 
   call remote_send(name, ":call server2client(expand('<client>'), 'another')\<CR>", 'g:myserverid')
-  let s:where = 151
   let peek_result = 'nothing'
   let r = remote_peek(g:myserverid, 'peek_result')
-  let s:where = 161
   " unpredictable whether the result is already avaialble.
   if r > 0
     call assert_equal('another', peek_result)
@@ -96,16 +77,11 @@ func Test_client_server()
   endif
   let g:peek_result = 'empty'
   call WaitFor('remote_peek(g:myserverid, "g:peek_result") > 0')
-  let s:where = 171
   call assert_equal('another', g:peek_result)
-  let s:where = 181
-  call assert_equal('another', remote_read(g:myserverid))
-  let s:where = 191
+  call assert_equal('another', remote_read(g:myserverid, 2))
 
   call remote_send(name, ":qa!\<CR>")
-  let s:where = 17
   call WaitFor('job_status(g:job) == "dead"')
-  let s:where = 18
   if job_status(g:job) != 'dead'
     call assert_report('Server did not exit')
     call job_stop(g:job, 'kill')
index 50b7c4a5d97b5f829f2f6c3ec46f81e23a549d38..75afd62d480477cf0d5cda16144b2ceb96b557e9 100644 (file)
@@ -764,6 +764,8 @@ static char *(features[]) =
 
 static int included_patches[] =
 {   /* Add new patch number below this line */
+/**/
+    492,
 /**/
     491,
 /**/
index dd571df3714f8a2b8dc961b3c0b0ce63495ba892..57866ef53e4d4b95a0e2f0580f0028a8ec9fec62 100644 (file)
--- a/src/vim.h
+++ b/src/vim.h
@@ -2506,7 +2506,9 @@ typedef enum {
 #  define ELAPSED_INIT(v) v = GetTickCount()
 #  define ELAPSED_FUNC(v) elapsed(v)
 #  define ELAPSED_TYPE DWORD
-    long elapsed(DWORD start_tick);
+#   ifndef PROTO
+     long elapsed(DWORD start_tick);
+#   endif
 # endif
 #endif