Skip to content

Commit

Permalink
job: fix race between channel close and job exit
Browse files Browse the repository at this point in the history
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 <pavel.borzenkov@gmail.com>
  • Loading branch information
pborzenkov committed Mar 28, 2017
1 parent 4119811 commit 5c721a2
Show file tree
Hide file tree
Showing 7 changed files with 28 additions and 59 deletions.
2 changes: 1 addition & 1 deletion autoload/go/cmd.vim
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion autoload/go/coverage.vim
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion autoload/go/def.vim
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
28 changes: 10 additions & 18 deletions autoload/go/guru.vim
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
Expand All @@ -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')
Expand Down
27 changes: 8 additions & 19 deletions autoload/go/job.vim
Original file line number Diff line number Diff line change
Expand Up @@ -30,36 +30,25 @@ 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")
endif
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
Expand Down Expand Up @@ -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
Expand Down
15 changes: 3 additions & 12 deletions autoload/go/lint.vim
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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)
Expand Down
11 changes: 4 additions & 7 deletions autoload/go/rename.vim
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 5c721a2

Please sign in to comment.