Permalink
Browse files

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 #1922)
            Implement options for term_start() to be able to test.
            Make term_wait() more reliable.
  • Loading branch information...
brammool committed Aug 3, 2017
1 parent 2f3a90a commit 3c3a80dc59ccc0e0aabb9c8bd58ea84a801dbfc1
Showing with 130 additions and 63 deletions.
  1. +2 −2 src/channel.c
  2. +102 −58 src/terminal.c
  3. +24 −3 src/testdir/test_terminal.vim
  4. +2 −0 src/version.c
View
@@ -4160,7 +4160,6 @@ get_job_options(typval_T *tv, jobopt_T *opt, int supported)
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 *opt_arg)
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. */
View
@@ -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);
@@ -273,6 +309,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".
*/
@@ -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;
@@ -1613,35 +1661,6 @@ term_get_attr(buf_T *buf, linenr_T lnum, int col)
return cell2attr(line->sb_cells + col);
}
/*
* 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.
*/
@@ -2089,12 +2108,19 @@ f_term_sendkeys(typval_T *argvars, typval_T *rettv)
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 *rettv UNUSED)
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, int cols, char_u *cmd)
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 rows, int cols)
* 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;
@@ -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()
View
@@ -769,6 +769,8 @@ static char *(features[]) =
static int included_patches[] =
{ /* Add new patch number below this line */
/**/
849,
/**/
848,
/**/

0 comments on commit 3c3a80d

Please sign in to comment.