From 5c721a27868d0c18797f37a1624f4ed998285e1e Mon Sep 17 00:00:00 2001 From: Pavel Borzenkov Date: Tue, 28 Mar 2017 16:54:55 +0000 Subject: [PATCH] job: fix race between channel close and job exit When channel's close_cb is called, the job this channel is assigned to may still be alive and the result status of the job will be set to FAILED (as no actual return code is available at that moment). This is easily reproduced for me on a single CPU Linux VPS. To fix this, check for job's status in exit_cb instead. Signed-off-by: Pavel Borzenkov --- autoload/go/cmd.vim | 2 +- autoload/go/coverage.vim | 2 +- autoload/go/def.vim | 2 +- autoload/go/guru.vim | 28 ++++++++++------------------ autoload/go/job.vim | 27 ++++++++------------------- autoload/go/lint.vim | 15 +++------------ autoload/go/rename.vim | 11 ++++------- 7 files changed, 28 insertions(+), 59 deletions(-) diff --git a/autoload/go/cmd.vim b/autoload/go/cmd.vim index 297fd8125d..f8de3fccb1 100644 --- a/autoload/go/cmd.vim +++ b/autoload/go/cmd.vim @@ -446,7 +446,7 @@ function s:cmd_job(args) abort let start_options = { \ 'callback': callbacks.callback, - \ 'close_cb': callbacks.close_cb, + \ 'exit_cb': callbacks.exit_cb, \ } " modify GOPATH if needed diff --git a/autoload/go/coverage.vim b/autoload/go/coverage.vim index 493d420579..fd2bb76e3c 100644 --- a/autoload/go/coverage.vim +++ b/autoload/go/coverage.vim @@ -288,7 +288,7 @@ function s:coverage_job(args) let start_options = { \ 'callback': callbacks.callback, - \ 'close_cb': callbacks.close_cb, + \ 'exit_cb': callbacks.exit_cb, \ } " modify GOPATH if needed diff --git a/autoload/go/def.vim b/autoload/go/def.vim index a3d5f84ac3..2f73ceeea9 100644 --- a/autoload/go/def.vim +++ b/autoload/go/def.vim @@ -303,7 +303,7 @@ function s:def_job(args) abort let start_options = { \ 'callback': callbacks.callback, - \ 'close_cb': callbacks.close_cb, + \ 'exit_cb': callbacks.exit_cb, \ } if &modified diff --git a/autoload/go/guru.vim b/autoload/go/guru.vim index 7edd6716a0..5cff49d8f6 100644 --- a/autoload/go/guru.vim +++ b/autoload/go/guru.vim @@ -155,21 +155,12 @@ function! s:async_guru(args) abort endif endif - function! s:close_cb(chan) closure - let messages = [] - while ch_status(a:chan, {'part': 'out'}) == 'buffered' - let msg = ch_read(a:chan, {'part': 'out'}) - call add(messages, msg) - endwhile - - while ch_status(a:chan, {'part': 'err'}) == 'buffered' - let msg = ch_read(a:chan, {'part': 'err'}) - call add(messages, msg) - endwhile - - let l:job = ch_getjob(a:chan) - let l:info = job_info(l:job) + let messages = [] + function! s:callback(chan, msg) closure + call add(messages, a:msg) + endfunction + function! s:exit_cb(job, exitval) closure let out = join(messages, "\n") let status = { @@ -178,21 +169,22 @@ function! s:async_guru(args) abort \ 'state': "finished", \ } - if l:info.exitval + if a:exitval let status.state = "failed" endif call go#statusline#Update(status_dir, status) if has_key(a:args, 'custom_parse') - call a:args.custom_parse(l:info.exitval, out) + call a:args.custom_parse(a:exitval, out) else - call s:parse_guru_output(l:info.exitval, out, a:args.mode) + call s:parse_guru_output(a:exitval, out, a:args.mode) endif endfunction let start_options = { - \ 'close_cb': funcref("s:close_cb"), + \ 'callback': funcref("s:callback"), + \ 'exit_cb': funcref("s:exit_cb"), \ } if has_key(result, 'stdin_content') diff --git a/autoload/go/job.vim b/autoload/go/job.vim index e60b2b45bb..6fb03af76b 100644 --- a/autoload/go/job.vim +++ b/autoload/go/job.vim @@ -30,28 +30,17 @@ function go#job#Spawn(args) call add(self.messages, a:msg) endfunction - function cbs.close_cb(chan) dict - let l:job = ch_getjob(a:chan) - let l:status = job_status(l:job) - - " the job might be in fail status, we assume by default it's failed. - " However if it's dead, we can use the real exitval - let exitval = 1 - if l:status == "dead" - let l:info = job_info(l:job) - let exitval = l:info.exitval - endif - + function cbs.exit_cb(job, exitval) dict if has_key(self, 'custom_cb') - call self.custom_cb(l:job, exitval, self.messages) + call self.custom_cb(a:job, a:exitval, self.messages) endif if has_key(self, 'error_info_cb') - call self.error_info_cb(l:job, exitval, self.messages) + call self.error_info_cb(a:job, a:exitval, self.messages) endif if get(g:, 'go_echo_command_info', 1) - if exitval == 0 + if a:exitval == 0 call go#util#EchoSuccess("SUCCESS") else call go#util#EchoError("FAILED") @@ -59,7 +48,7 @@ function go#job#Spawn(args) endif let l:listtype = go#list#Type("quickfix") - if exitval == 0 + if a:exitval == 0 call go#list#Clean(l:listtype) call go#list#Window(l:listtype) return @@ -99,9 +88,9 @@ function go#job#Spawn(args) let cbs.callback = a:args.callback endif - " override close callback handler if user provided it - if has_key(a:args, 'close_cb') - let cbs.close_cb = a:args.close_cb + " override exit callback handler if user provided it + if has_key(a:args, 'exit_cb') + let cbs.exit_cb = a:args.exit_cb endif return cbs diff --git a/autoload/go/lint.vim b/autoload/go/lint.vim index 84d8ba4560..588741a52a 100644 --- a/autoload/go/lint.vim +++ b/autoload/go/lint.vim @@ -252,23 +252,14 @@ function s:lint_job(args) copen endfunction - function! s:close_cb(chan) closure - let l:job = ch_getjob(a:chan) - let l:status = job_status(l:job) - - let exitval = 1 - if l:status == "dead" - let l:info = job_info(l:job) - let exitval = l:info.exitval - endif - + function! s:exit_cb(job, exitval) closure let status = { \ 'desc': 'last status', \ 'type': "gometaliner", \ 'state': "finished", \ } - if exitval + if a:exitval let status.state = "failed" endif @@ -297,7 +288,7 @@ function s:lint_job(args) let start_options = { \ 'callback': funcref("s:callback"), - \ 'close_cb': funcref("s:close_cb"), + \ 'exit_cb': funcref("s:exit_cb"), \ } call job_start(a:args.cmd, start_options) diff --git a/autoload/go/rename.vim b/autoload/go/rename.vim index ac2f0181c5..3ca47d58b9 100644 --- a/autoload/go/rename.vim +++ b/autoload/go/rename.vim @@ -71,28 +71,25 @@ function s:rename_job(args) let status_dir = expand('%:p:h') - function! s:close_cb(chan) closure - let l:job = ch_getjob(a:chan) - let l:info = job_info(l:job) - + function! s:exit_cb(job, exitval) closure let status = { \ 'desc': 'last status', \ 'type': "gorename", \ 'state': "finished", \ } - if l:info.exitval + if a:exitval let status.state = "failed" endif call go#statusline#Update(status_dir, status) - call s:parse_errors(l:info.exitval, a:args.bang, messages) + call s:parse_errors(a:exitval, a:args.bang, messages) endfunction let start_options = { \ 'callback': funcref("s:callback"), - \ 'close_cb': funcref("s:close_cb"), + \ 'exit_cb': funcref("s:exit_cb"), \ } " modify GOPATH if needed