Mercurial > vim
changeset 11211:71311d899b42 v8.0.0492
patch 8.0.0492: a failing client-server request can make Vim hang
commit https://github.com/vim/vim/commit/81b9d0bd5c705815e903e671e81b0b05828efd9c
Author: Bram Moolenaar <Bram@vim.org>
Date: Sun Mar 19 21:20:53 2017 +0100
patch 8.0.0492: a failing client-server request can make Vim hang
Problem: A failing client-server request can make Vim hang.
Solution: Add a timeout argument to functions that wait.
| author | Christian Brabandt <cb@256bit.org> |
|---|---|
| date | Sun, 19 Mar 2017 21:30:05 +0100 |
| parents | 5ffe3a7f9bd7 |
| children | 22a64df8374b |
| files | 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 |
| diffstat | 10 files changed, 84 insertions(+), 76 deletions(-) [+] |
line wrap: on
line diff
--- a/runtime/doc/eval.txt +++ b/runtime/doc/eval.txt @@ -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}]) *r :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}
--- a/src/evalfunc.c +++ b/src/evalfunc.c @@ -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_ 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_ 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_ 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, 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, 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")); }
--- a/src/if_xcmdsrv.c +++ b/src/if_xcmdsrv.c @@ -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) {
--- a/src/main.c +++ b/src/main.c @@ -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)
--- a/src/os_mswin.c +++ b/src/os_mswin.c @@ -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, i * 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_re #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. */
--- a/src/proto/if_xcmdsrv.pro +++ b/src/proto/if_xcmdsrv.pro @@ -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);
--- a/src/proto/os_mswin.pro +++ b/src/proto/os_mswin.pro @@ -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);
--- a/src/testdir/test_clientserver.vim +++ b/src/testdir/test_clientserver.vim @@ -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')