From 1507fdf2b8a3709276eab88cae9f8f41fda292a7 Mon Sep 17 00:00:00 2001 From: Daniel Hahler Date: Thu, 10 Oct 2019 18:04:15 +0200 Subject: [PATCH 1/8] ChildProcessStream: store exitcode/exitsignal --- nvim/child_process_stream.lua | 4 +++- nvim/session.lua | 7 ++++++- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/nvim/child_process_stream.lua b/nvim/child_process_stream.lua index 5125a0e..fc66334 100644 --- a/nvim/child_process_stream.lua +++ b/nvim/child_process_stream.lua @@ -19,7 +19,9 @@ function ChildProcessStream.spawn(argv, env) stdio = {self._child_stdin, self._child_stdout, 2}, args = args, env = env, - }, function() + }, function(code, signal) + self.exitcode = code + self.exitsignal = signal self:close() end) diff --git a/nvim/session.lua b/nvim/session.lua index 25574dc..5a7b2cd 100644 --- a/nvim/session.lua +++ b/nvim/session.lua @@ -175,7 +175,12 @@ function Session:_run(request_cb, notification_cb, timeout) self._prepare:stop() end) end - self._msgpack_rpc_stream:read_start(request_cb, notification_cb, uv.stop) + self._msgpack_rpc_stream:read_start(request_cb, notification_cb, function() + uv.run() -- run the loop to get exitcode from child process. + self.child_exit = self._msgpack_rpc_stream._stream.exitcode + self.child_signal = self._msgpack_rpc_stream._stream.exitsignal + uv.stop() + end) uv.run() self._prepare:stop() self._timer:stop() From bb064501203c5717197f8871522c19f0d5829e57 Mon Sep 17 00:00:00 2001 From: Daniel Hahler Date: Thu, 10 Oct 2019 19:13:15 +0200 Subject: [PATCH 2/8] ChildProcessStream: do not close _proc, but wait for it (exit_cb) --- nvim/child_process_stream.lua | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/nvim/child_process_stream.lua b/nvim/child_process_stream.lua index fc66334..67fa386 100644 --- a/nvim/child_process_stream.lua +++ b/nvim/child_process_stream.lua @@ -61,8 +61,8 @@ function ChildProcessStream:close(signal) if type(signal) == 'string' then self._proc:kill('sig'..signal) end - self._proc:close() - uv.run('nowait') + + uv.run() native.pid_wait(self._pid) end From a77dffc5532163e30c435d03139b38b85d5f8098 Mon Sep 17 00:00:00 2001 From: Daniel Hahler Date: Thu, 10 Oct 2019 19:13:38 +0200 Subject: [PATCH 3/8] Session:close: fetch child_exit if not there already --- nvim/session.lua | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/nvim/session.lua b/nvim/session.lua index 5a7b2cd..ff4c5f5 100644 --- a/nvim/session.lua +++ b/nvim/session.lua @@ -135,6 +135,12 @@ function Session:close(signal) if not self._timer:is_closing() then self._timer:close() end if not self._prepare:is_closing() then self._prepare:close() end self._msgpack_rpc_stream:close(signal) + + if not self.child_exit then + uv.run('nowait') -- run the loop to get exitcode from child process. + self.child_exit = self._msgpack_rpc_stream._stream.exitcode + self.child_signal = self._msgpack_rpc_stream._stream.exitsignal + end end function Session:_yielding_request(method, args) From 33a83ff3b3c6dcce64f9868a9caeeb1f7e92e95f Mon Sep 17 00:00:00 2001 From: Daniel Hahler Date: Thu, 10 Oct 2019 19:20:22 +0200 Subject: [PATCH 4/8] Revert "ChildProcessStream: store exitcode/exitsignal" This reverts commit 7fa9fa55721f1f9bc14196e295f41fc59937f954. --- nvim/session.lua | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/nvim/session.lua b/nvim/session.lua index ff4c5f5..a6cd38a 100644 --- a/nvim/session.lua +++ b/nvim/session.lua @@ -181,12 +181,7 @@ function Session:_run(request_cb, notification_cb, timeout) self._prepare:stop() end) end - self._msgpack_rpc_stream:read_start(request_cb, notification_cb, function() - uv.run() -- run the loop to get exitcode from child process. - self.child_exit = self._msgpack_rpc_stream._stream.exitcode - self.child_signal = self._msgpack_rpc_stream._stream.exitsignal - uv.stop() - end) + self._msgpack_rpc_stream:read_start(request_cb, notification_cb, uv.stop) uv.run() self._prepare:stop() self._timer:stop() From 7747b9808593a5b1998ce2392eed5b4a7c44c653 Mon Sep 17 00:00:00 2001 From: Daniel Hahler Date: Fri, 11 Oct 2019 03:55:16 +0200 Subject: [PATCH 5/8] Revert "Revert "ChildProcessStream: store exitcode/exitsignal"" This reverts commit 6736442472e1a9ca516e77b6dc899eb9cc489616. --- nvim/session.lua | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/nvim/session.lua b/nvim/session.lua index a6cd38a..ff4c5f5 100644 --- a/nvim/session.lua +++ b/nvim/session.lua @@ -181,7 +181,12 @@ function Session:_run(request_cb, notification_cb, timeout) self._prepare:stop() end) end - self._msgpack_rpc_stream:read_start(request_cb, notification_cb, uv.stop) + self._msgpack_rpc_stream:read_start(request_cb, notification_cb, function() + uv.run() -- run the loop to get exitcode from child process. + self.child_exit = self._msgpack_rpc_stream._stream.exitcode + self.child_signal = self._msgpack_rpc_stream._stream.exitsignal + uv.stop() + end) uv.run() self._prepare:stop() self._timer:stop() From 059217650bd26f0d681a0c742759b591a67da5ad Mon Sep 17 00:00:00 2001 From: Daniel Hahler Date: Fri, 11 Oct 2019 14:16:42 +0200 Subject: [PATCH 6/8] assertion --- nvim/child_process_stream.lua | 1 + 1 file changed, 1 insertion(+) diff --git a/nvim/child_process_stream.lua b/nvim/child_process_stream.lua index 67fa386..d7c35e5 100644 --- a/nvim/child_process_stream.lua +++ b/nvim/child_process_stream.lua @@ -63,6 +63,7 @@ function ChildProcessStream:close(signal) end uv.run() + assert(self.exitcode) native.pid_wait(self._pid) end From c77c7b02cd5f7448c628d42b572dd876d6396a6e Mon Sep 17 00:00:00 2001 From: Daniel Hahler Date: Mon, 14 Oct 2019 16:21:04 +0200 Subject: [PATCH 7/8] ChildProcessStream:close: uv.run: once --- nvim/child_process_stream.lua | 4 +++- test/session_spec.lua | 8 ++++++++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/nvim/child_process_stream.lua b/nvim/child_process_stream.lua index d7c35e5..3d1fc1d 100644 --- a/nvim/child_process_stream.lua +++ b/nvim/child_process_stream.lua @@ -62,7 +62,9 @@ function ChildProcessStream:close(signal) self._proc:kill('sig'..signal) end - uv.run() + while not self.exitcode do + uv.run('once') + end assert(self.exitcode) native.pid_wait(self._pid) end diff --git a/test/session_spec.lua b/test/session_spec.lua index 73fbfef..cd37db5 100644 --- a/test/session_spec.lua +++ b/test/session_spec.lua @@ -257,3 +257,11 @@ describe('stdio', function() assert.are.same({'notification', 'd', {6, 7}}, session:next_message()) end) end) + +it('closing session does not hang with active loop', function() + local cmd = {nvim_prog, '-u', 'NONE', '--embed', '--headless'} + local session1 = Session.new(ChildProcessStream.spawn(cmd)) + local session2 = Session.new(ChildProcessStream.spawn(cmd)) + session1:close() + session2:close() +end) From ecd6601d3073eb8f7130f0809b00e4c7b63f5e7a Mon Sep 17 00:00:00 2001 From: Daniel Hahler Date: Fri, 29 Nov 2019 05:12:50 +0100 Subject: [PATCH 8/8] tests --- test/session_spec.lua | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/test/session_spec.lua b/test/session_spec.lua index cd37db5..6c51ab1 100644 --- a/test/session_spec.lua +++ b/test/session_spec.lua @@ -153,6 +153,8 @@ local function test_session(description, session_factory, session_destroy) session_destroy() else session:close() + assert.are.equal(0, session.child_exit) + assert.are.equal(0, session.child_signal) end closed = true end) @@ -179,6 +181,8 @@ test_session(string.format("Session using SocketStream [%s]", socket_file), func return socket_session end, function () child_session:close() + assert.are.equal(0, child_session.child_exit) + assert.are.equal(0, child_session.child_signal) socket_session:close() -- clean up leftovers if something goes wrong local fd = io.open(socket_file) @@ -218,6 +222,8 @@ test_session("Session using TcpStream", function () return tcp_session end, function () child_session:close() + assert.are.equal(0, child_session.child_exit) + assert.are.equal(0, child_session.child_signal) tcp_session:close() end)