diff options
-rw-r--r-- | src/nvim/os/job.c | 107 | ||||
-rw-r--r-- | test/functional/shell/viml_system_spec.lua | 29 |
2 files changed, 78 insertions, 58 deletions
diff --git a/src/nvim/os/job.c b/src/nvim/os/job.c index 4c01829159..947538af4b 100644 --- a/src/nvim/os/job.c +++ b/src/nvim/os/job.c @@ -21,6 +21,21 @@ #define MAX_RUNNING_JOBS 100 #define JOB_BUFFER_SIZE 0xFFFF +#define close_job_stream(job, stream, type) \ + do { \ + if (job->stream) { \ + type##stream_free(job->stream); \ + job->stream = NULL; \ + if (!uv_is_closing((uv_handle_t *)&job->proc_std##stream)) { \ + uv_close((uv_handle_t *)&job->proc_std##stream, close_cb); \ + } \ + } \ + } while (0) + +#define close_job_in(job) close_job_stream(job, in, w) +#define close_job_out(job) close_job_stream(job, out, r) +#define close_job_err(job) close_job_stream(job, err, r) + struct job { // Job id the index in the job table plus one. int id; @@ -28,13 +43,9 @@ struct job { int64_t status; // Number of polls after a SIGTERM that will trigger a SIGKILL int exit_timeout; - // exit_cb may be called while there's still pending data from stdout/stderr. - // We use this reference count to ensure the JobExit event is only emitted - // when stdout/stderr are drained - int pending_refs; - // Same as above, but for freeing the job memory which contains - // libuv handles. Only after all are closed the job can be safely freed. - int pending_closes; + // Number of references to the job. The job resources will only be freed by + // close_cb when this is 0 + int refcount; // If the job was already stopped bool stopped; // Data associated with the job @@ -164,8 +175,7 @@ Job *job_start(char **argv, job->id = i + 1; *status = job->id; job->status = -1; - job->pending_refs = 3; - job->pending_closes = 4; + job->refcount = 4; job->data = data; job->stdout_cb = stdout_cb; job->stderr_cb = stderr_cb; @@ -206,7 +216,9 @@ Job *job_start(char **argv, // Spawn the job if (uv_spawn(uv_default_loop(), &job->proc, &job->proc_opts) != 0) { - free_job(job); + close_job_in(job); + close_job_out(job); + close_job_err(job); *status = -1; return NULL; } @@ -272,15 +284,14 @@ int job_wait(Job *job, int ms) FUNC_ATTR_NONNULL_ALL int old_mode = cur_tmode; settmode(TMODE_COOK); - // Increase pending_refs to stop the exit_cb from being called, which - // could result in the job being freed before we have a chance - // to get the status. - job->pending_refs++; + // Increase refcount to stop the job from being freed before we have a + // chance to get the status. + job->refcount++; event_poll_until(ms, // Until... got_int || // interrupted by the user - job->pending_refs == 1); // job exited - job->pending_refs--; + job->refcount == 1); // job exited + job->refcount--; // we'll assume that a user frantically hitting interrupt doesn't like // the current job. Signal that it has to be killed. @@ -291,9 +302,10 @@ int job_wait(Job *job, int ms) FUNC_ATTR_NONNULL_ALL settmode(old_mode); - if (!job->pending_refs) { + if (!job->refcount) { int status = (int) job->status; - job_exit_callback(job); + // Manually invoke close_cb to free the job resources + close_cb((uv_handle_t *)&job->proc); return status; } @@ -312,15 +324,7 @@ int job_wait(Job *job, int ms) FUNC_ATTR_NONNULL_ALL /// @param job The job instance void job_close_in(Job *job) FUNC_ATTR_NONNULL_ALL { - if (!job->in) { - return; - } - - // let other functions in the job module know that the in pipe is no more - wstream_free(job->in); - job->in = NULL; - - uv_close((uv_handle_t *)&job->proc_stdin, close_cb); + close_job_in(job); } /// All writes that complete after calling this function will be reported @@ -379,9 +383,6 @@ static void job_exit_callback(Job *job) job->exit_cb(job, job->data); } - // Free the job resources - free_job(job); - // Stop polling job status if this was the last job_count--; if (job_count == 0) { @@ -394,16 +395,6 @@ static bool is_alive(Job *job) return uv_process_kill(&job->proc, 0) == 0; } -static void free_job(Job *job) -{ - uv_close((uv_handle_t *)&job->proc_stdout, close_cb); - if (job->in) { - uv_close((uv_handle_t *)&job->proc_stdin, close_cb); - } - uv_close((uv_handle_t *)&job->proc_stderr, close_cb); - uv_close((uv_handle_t *)&job->proc, close_cb); -} - /// Iterates the table, sending SIGTERM to stopped jobs and SIGKILL to those /// that didn't die from SIGTERM after a while(exit_timeout is 0). static void job_prepare_cb(uv_prepare_t *handle) @@ -433,12 +424,14 @@ static void read_cb(RStream *rstream, void *data, bool eof) if (rstream == job->out) { job->stdout_cb(rstream, data, eof); + if (eof) { + close_job_out(job); + } } else { job->stderr_cb(rstream, data, eof); - } - - if (eof && --job->pending_refs == 0) { - job_exit_callback(job); + if (eof) { + close_job_err(job); + } } } @@ -448,31 +441,29 @@ static void exit_cb(uv_process_t *proc, int64_t status, int term_signal) Job *job = handle_get_job((uv_handle_t *)proc); job->status = status; - if (--job->pending_refs == 0) { - job_exit_callback(job); - } + uv_close((uv_handle_t *)&job->proc, close_cb); } static void close_cb(uv_handle_t *handle) { Job *job = handle_get_job(handle); - if (--job->pending_closes == 0) { - // Only free the job memory after all the associated handles are properly - // closed by libuv - rstream_free(job->out); - rstream_free(job->err); - if (job->in) { - wstream_free(job->in); - } + if (handle == (uv_handle_t *)&job->proc) { + // Make sure all streams are properly closed to trigger callback invocation + // when job->proc is closed + close_job_in(job); + close_job_out(job); + close_job_err(job); + } - // Free data memory of process and pipe handles, that was allocated - // by handle_set_job in job_start. + if (--job->refcount == 0) { + // Invoke the exit_cb + job_exit_callback(job); + // Free all memory allocated for the job free(job->proc.data); free(job->proc_stdin.data); free(job->proc_stdout.data); free(job->proc_stderr.data); - shell_free_argv(job->proc_opts.args); free(job); } diff --git a/test/functional/shell/viml_system_spec.lua b/test/functional/shell/viml_system_spec.lua index 6dde2ef538..9ed1016f73 100644 --- a/test/functional/shell/viml_system_spec.lua +++ b/test/functional/shell/viml_system_spec.lua @@ -19,6 +19,15 @@ local function delete_file(name) end end +-- Some tests require the xclip program and a x server. +local xclip = nil +do + if os.getenv('DISPLAY') then + local proc = io.popen('which xclip') + xclip = proc:read() + proc:close() + end +end describe('system()', function() before_each(clear) @@ -85,6 +94,15 @@ describe('system()', function() end) end) end) + + if xclip then + describe("with a program that doesn't close stdout", function() + it('will exit properly after passing input', function() + eq(nil, eval([[system('xclip -i -selection clipboard', 'clip-data')]])) + eq('clip-data', eval([[system('xclip -o -selection clipboard')]])) + end) + end) + end end) describe('systemlist()', function() @@ -140,4 +158,15 @@ describe('systemlist()', function() end) end) end) + + if xclip then + describe("with a program that doesn't close stdout", function() + it('will exit properly after passing input', function() + eq(nil, eval( + "systemlist('xclip -i -selection clipboard', ['clip', 'data'])")) + eq({'clip', 'data'}, eval( + "systemlist('xclip -o -selection clipboard')")) + end) + end) + end end) |