Mercurial > vim
changeset 11939:ef1febf04d03 v8.0.0849
patch 8.0.0849: crash when job exit callback wipes the terminal
commit https://github.com/vim/vim/commit/3c3a80dc59ccc0e0aabb9c8bd58ea84a801dbfc1
Author: Bram Moolenaar <Bram@vim.org>
Date: Thu Aug 3 17:06:45 2017 +0200
patch 8.0.0849: crash when job exit callback wipes the terminal
Problem: Crash when job exit callback wipes the terminal.
Solution: Check for b_term to be NULL. (Yasuhiro Matsumoto, closes https://github.com/vim/vim/issues/1922)
Implement options for term_start() to be able to test.
Make term_wait() more reliable.
| author | Christian Brabandt <cb@256bit.org> |
|---|---|
| date | Thu, 03 Aug 2017 17:15:04 +0200 |
| parents | 4aeeb22b875d |
| children | 0372e70a8358 |
| files | src/channel.c src/terminal.c src/testdir/test_terminal.vim src/version.c |
| diffstat | 4 files changed, 130 insertions(+), 63 deletions(-) [+] |
line wrap: on
line diff
--- a/src/channel.c +++ b/src/channel.c @@ -4160,7 +4160,6 @@ get_job_options(typval_T *tv, jobopt_T * hashitem_T *hi; ch_part_T part; - opt->jo_set = 0; if (tv->v_type == VAR_UNKNOWN) return OK; if (tv->v_type != VAR_DICT) @@ -4616,6 +4615,7 @@ job_cleanup(job_T *job) int dummy; /* Invoke the exit callback. Make sure the refcount is > 0. */ + ch_log(job->jv_channel, "Invoking exit callback %s", job->jv_exit_cb); ++job->jv_refcount; argv[0].v_type = VAR_JOB; argv[0].vval.v_job = job; @@ -4888,7 +4888,7 @@ job_start(typval_T *argvars, jobopt_T *o if (get_job_options(&argvars[1], &opt, JO_MODE_ALL + JO_CB_ALL + JO_TIMEOUT_ALL + JO_STOPONEXIT + JO_EXIT_CB + JO_OUT_IO + JO_BLOCK_WRITE) == FAIL) - goto theend; + goto theend; } /* Check that when io is "file" that there is a file name. */
--- a/src/terminal.c +++ b/src/terminal.c @@ -40,6 +40,7 @@ * - MS-Windows: no redraw for 'updatetime' #1915 * - in bash mouse clicks are inserting characters. * - mouse scroll: when over other window, scroll that window. + * - add argument to term_wait() for waiting time. * - For the scrollback buffer store lines in the buffer, only attributes in * tl_scrollback. * - When the job ends: @@ -146,7 +147,7 @@ static term_T *first_term = NULL; /* * Functions with separate implementation for MS-Windows and Unix-like systems. */ -static int term_and_job_init(term_T *term, int rows, int cols, char_u *cmd); +static int term_and_job_init(term_T *term, int rows, int cols, char_u *cmd, jobopt_T *opt); static void term_report_winsize(term_T *term, int rows, int cols); static void term_free_vterm(term_T *term); @@ -185,15 +186,49 @@ set_term_and_win_size(term_T *term) } /* - * ":terminal": open a terminal window and execute a job in it. + * Initialize job options for a terminal job. + * Caller may overrule some of them. */ - void -ex_terminal(exarg_T *eap) + static void +init_job_options(jobopt_T *opt) +{ + clear_job_options(opt); + + opt->jo_mode = MODE_RAW; + opt->jo_out_mode = MODE_RAW; + opt->jo_err_mode = MODE_RAW; + opt->jo_set = JO_MODE | JO_OUT_MODE | JO_ERR_MODE; + + opt->jo_io[PART_OUT] = JIO_BUFFER; + opt->jo_io[PART_ERR] = JIO_BUFFER; + opt->jo_set |= JO_OUT_IO + JO_ERR_IO; + + opt->jo_modifiable[PART_OUT] = 0; + opt->jo_modifiable[PART_ERR] = 0; + opt->jo_set |= JO_OUT_MODIFIABLE + JO_ERR_MODIFIABLE; + + opt->jo_set |= JO_OUT_BUF + JO_ERR_BUF; +} + +/* + * Set job options mandatory for a terminal job. + */ + static void +setup_job_options(jobopt_T *opt, int rows, int cols) +{ + opt->jo_io_buf[PART_OUT] = curbuf->b_fnum; + opt->jo_io_buf[PART_ERR] = curbuf->b_fnum; + opt->jo_pty = TRUE; + opt->jo_term_rows = rows; + opt->jo_term_cols = cols; +} + + static void +term_start(char_u *cmd, jobopt_T *opt) { exarg_T split_ea; win_T *old_curwin = curwin; term_T *term; - char_u *cmd = eap->arg; if (check_restricted() || check_secure()) return; @@ -256,9 +291,10 @@ ex_terminal(exarg_T *eap) (char_u *)"terminal", OPT_FREE|OPT_LOCAL, 0); set_term_and_win_size(term); + setup_job_options(opt, term->tl_rows, term->tl_cols); /* System dependent: setup the vterm and start the job in it. */ - if (term_and_job_init(term, term->tl_rows, term->tl_cols, cmd) == OK) + if (term_and_job_init(term, term->tl_rows, term->tl_cols, cmd, opt) == OK) { /* store the size we ended up with */ vterm_get_size(term->tl_vterm, &term->tl_rows, &term->tl_cols); @@ -274,6 +310,20 @@ ex_terminal(exarg_T *eap) } /* + * ":terminal": open a terminal window and execute a job in it. + */ + void +ex_terminal(exarg_T *eap) +{ + jobopt_T opt; + + init_job_options(&opt); + /* TODO: get options from before the command */ + + term_start(eap->arg, &opt); +} + +/* * Free the scrollback buffer for "term". */ static void @@ -965,8 +1015,7 @@ terminal_loop(void) update_cursor(curbuf->b_term, FALSE); c = term_vgetc(); - if (curbuf->b_term->tl_vterm == NULL - || !term_job_running(curbuf->b_term)) + if (!term_use_loop()) /* job finished while waiting for a character */ break; @@ -993,8 +1042,7 @@ terminal_loop(void) #ifdef FEAT_CMDL_INFO clear_showcmd(); #endif - if (curbuf->b_term->tl_vterm == NULL - || !term_job_running(curbuf->b_term)) + if (!term_use_loop()) /* job finished while waiting for a character */ break; @@ -1614,35 +1662,6 @@ term_get_attr(buf_T *buf, linenr_T lnum, } /* - * Set job options common for Unix and MS-Windows. - */ - static void -setup_job_options(jobopt_T *opt, int rows, int cols) -{ - clear_job_options(opt); - opt->jo_mode = MODE_RAW; - opt->jo_out_mode = MODE_RAW; - opt->jo_err_mode = MODE_RAW; - opt->jo_set = JO_MODE | JO_OUT_MODE | JO_ERR_MODE; - - opt->jo_io[PART_OUT] = JIO_BUFFER; - opt->jo_io[PART_ERR] = JIO_BUFFER; - opt->jo_set |= JO_OUT_IO + JO_ERR_IO; - - opt->jo_modifiable[PART_OUT] = 0; - opt->jo_modifiable[PART_ERR] = 0; - opt->jo_set |= JO_OUT_MODIFIABLE + JO_ERR_MODIFIABLE; - - opt->jo_io_buf[PART_OUT] = curbuf->b_fnum; - opt->jo_io_buf[PART_ERR] = curbuf->b_fnum; - opt->jo_pty = TRUE; - opt->jo_set |= JO_OUT_BUF + JO_ERR_BUF; - - opt->jo_term_rows = rows; - opt->jo_term_cols = cols; -} - -/* * Create a new vterm and initialize it. */ static void @@ -2089,12 +2108,19 @@ f_term_sendkeys(typval_T *argvars, typva f_term_start(typval_T *argvars, typval_T *rettv) { char_u *cmd = get_tv_string_chk(&argvars[0]); - exarg_T ea; + jobopt_T opt; if (cmd == NULL) return; - ea.arg = cmd; - ex_terminal(&ea); + init_job_options(&opt); + /* TODO: allow more job options */ + if (argvars[1].v_type != VAR_UNKNOWN + && get_job_options(&argvars[1], &opt, + JO_TIMEOUT_ALL + JO_STOPONEXIT + + JO_EXIT_CB + JO_CLOSE_CALLBACK) == FAIL) + return; + + term_start(cmd, &opt); if (curbuf->b_term != NULL) rettv->vval.v_number = curbuf->b_fnum; @@ -2109,19 +2135,40 @@ f_term_wait(typval_T *argvars, typval_T buf_T *buf = term_get_buf(argvars); if (buf == NULL) + { + ch_log(NULL, "term_wait(): invalid argument"); return; + } /* Get the job status, this will detect a job that finished. */ - if (buf->b_term->tl_job != NULL) - (void)job_status(buf->b_term->tl_job); + if (buf->b_term->tl_job == NULL + || STRCMP(job_status(buf->b_term->tl_job), "dead") == 0) + { + /* The job is dead, keep reading channel I/O until the channel is + * closed. */ + while (buf->b_term != NULL && !buf->b_term->tl_channel_closed) + { + mch_char_avail(); + parse_queued_messages(); + ui_delay(10L, FALSE); + } + mch_char_avail(); + parse_queued_messages(); + } + else + { + mch_char_avail(); + parse_queued_messages(); - /* Check for any pending channel I/O. */ - vpeekc_any(); - ui_delay(10L, FALSE); + /* Wait for 10 msec for any channel I/O. */ + /* TODO: use delay from optional argument */ + ui_delay(10L, TRUE); + mch_char_avail(); - /* Flushing messages on channels is hopefully sufficient. - * TODO: is there a better way? */ - parse_queued_messages(); + /* Flushing messages on channels is hopefully sufficient. + * TODO: is there a better way? */ + parse_queued_messages(); + } } # ifdef WIN3264 @@ -2209,12 +2256,11 @@ dyn_winpty_init(void) * Return OK or FAIL. */ static int -term_and_job_init(term_T *term, int rows, int cols, char_u *cmd) +term_and_job_init(term_T *term, int rows, int cols, char_u *cmd, jobopt_T *opt) { WCHAR *p; channel_T *channel = NULL; job_T *job = NULL; - jobopt_T opt; DWORD error; HANDLE jo = NULL, child_process_handle, child_thread_handle; void *winpty_err; @@ -2298,8 +2344,7 @@ term_and_job_init(term_T *term, int rows create_vterm(term, rows, cols); - setup_job_options(&opt, rows, cols); - channel_set_job(channel, job, &opt); + channel_set_job(channel, job, opt); job->jv_channel = channel; job->jv_proc_info.hProcess = child_process_handle; @@ -2381,18 +2426,17 @@ term_report_winsize(term_T *term, int ro * Return OK or FAIL. */ static int -term_and_job_init(term_T *term, int rows, int cols, char_u *cmd) +term_and_job_init(term_T *term, int rows, int cols, char_u *cmd, jobopt_T *opt) { typval_T argvars[2]; - jobopt_T opt; create_vterm(term, rows, cols); /* TODO: if the command is "NONE" only create a pty. */ argvars[0].v_type = VAR_STRING; argvars[0].vval.v_string = cmd; - setup_job_options(&opt, rows, cols); - term->tl_job = job_start(argvars, &opt); + + term->tl_job = job_start(argvars, opt); if (term->tl_job != NULL) ++term->tl_job->jv_refcount;
--- a/src/testdir/test_terminal.vim +++ b/src/testdir/test_terminal.vim @@ -86,6 +86,23 @@ func Test_terminal_hide_buffer() unlet g:job endfunc +func! s:Nasty_exit_cb(job, st) + exe g:buf . 'bwipe!' + let g:buf = 0 +endfunc + +func Test_terminal_nasty_cb() + let cmd = Get_cat_cmd() + let g:buf = term_start(cmd, {'exit_cb': function('s:Nasty_exit_cb')}) + let g:job = term_getjob(g:buf) + + call WaitFor('job_status(g:job) == "dead"') + call WaitFor('g:buf == 0') + unlet g:buf + unlet g:job + call delete('Xtext') +endfunc + func Check_123(buf) let l = term_scrape(a:buf, 0) call assert_true(len(l) == 0) @@ -113,13 +130,17 @@ func Check_123(buf) call assert_equal('123', l) endfunc -func Test_terminal_scrape() +func Get_cat_cmd() if has('win32') - let cmd = 'cmd /c "cls && color 2 && echo 123"' + return 'cmd /c "cls && color 2 && echo 123"' else call writefile(["\<Esc>[32m123"], 'Xtext') - let cmd = "cat Xtext" + return "cat Xtext" endif +endfunc + +func Test_terminal_scrape() + let cmd = Get_cat_cmd() let buf = term_start(cmd) let termlist = term_list()