From ca4633bfe4d0f58bd5fb7343d282fafb71f3a3ee Mon Sep 17 00:00:00 2001 From: "Justin M. Keyes" Date: Sun, 2 Jul 2017 00:30:00 +0200 Subject: ci/quickbuild: XXX: disable server_requests test (#6851) Temporarily disable this test which hangs quickbuild. From #6905: The hang occurs when calling nvim_set_current_line. References #6594 5a151555c8dce70bbf235e7f6d5bd1ced5e7c46c --- test/functional/api/server_requests_spec.lua | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) (limited to 'test/functional/api/server_requests_spec.lua') diff --git a/test/functional/api/server_requests_spec.lua b/test/functional/api/server_requests_spec.lua index cf15062325..6a32f979ea 100644 --- a/test/functional/api/server_requests_spec.lua +++ b/test/functional/api/server_requests_spec.lua @@ -282,8 +282,13 @@ describe('server -> client', function() end) end) - describe('when connecting to its own pipe adress', function() - it('it does not deadlock', function() + describe('connecting to its own pipe address', function() + it('does not deadlock', function() + if not os.getenv("TRAVIS") and helpers.os_name() == "osx" then + -- It does, in fact, deadlock on QuickBuild. #6851 + pending("deadlocks on QuickBuild", function() end) + return + end local address = funcs.serverlist()[1] local first = string.sub(address,1,1) ok(first == '/' or first == '\\') -- cgit From af993da4351d579aa24de08d806c8c1b90813106 Mon Sep 17 00:00:00 2001 From: "Justin M. Keyes" Date: Thu, 27 Jul 2017 01:08:50 +0200 Subject: rpc: close channel if stream was closed f_jobstop()/f_rpcstop() .. process_stop() .. process_close_in(proc) closes the write-stream of a RPC channel. But there might be a pending RPC notification on the queue, which may get processed just before the channel is closed. To handle that case, check the Stream.closed in channel.c:receive_msgpack(). Before this change, the above scenario could trigger this assert(!stream->closed) in wstream_write(): 0x00007f96e1cd3428 in __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:54 0x00007f96e1cd502a in __GI_abort () at abort.c:89 0x00007f96e1ccbbd7 in __assert_fail_base (fmt=, assertion=assertion@entry=0x768f9b "!stream->closed", file=file@entry=0x768f70 "../src/nvim/event/wstream.c", line=line@entry=77, function=function@entry=0x768fb0 <__PRETTY_FUNCTION__.13735> "wstream_write") at assert.c:92 0x00007f96e1ccbc82 in __GI___assert_fail (assertion=0x768f9b "!stream->closed", file=0x768f70 "../src/nvim/event/wstream.c", line=77, function=0x768fb0 <__PRETTY_FUNCTION__.13735> "wstream_write") at assert.c:101 0x00000000004d2c1f in wstream_write (stream=0x7f96e0a35078, buffer=0x7f96e09f9b40) at ../src/nvim/event/wstream.c:77 0x00000000005857b2 in channel_write (channel=0x7f96e0ae5800, buffer=0x7f96e09f9b40) at ../src/nvim/msgpack_rpc/channel.c:551 0x000000000058567d in on_request_event (argv=0x7ffed792efa0) at ../src/nvim/msgpack_rpc/channel.c:523 0x00000000005854c8 in handle_request (channel=0x7f96e0ae5800, request=0x7ffed792f1b8) at ../src/nvim/msgpack_rpc/channel.c:503 0x00000000005850cb in parse_msgpack (channel=0x7f96e0ae5800) at ../src/nvim/msgpack_rpc/channel.c:423 0x0000000000584f90 in receive_msgpack (stream=0x7f96e0a35218, rbuf=0x7f96e0d1d4c0, c=22, data=0x7f96e0ae5800, eof=false) at ../src/nvim/msgpack_rpc/channel.c:389 0x00000000004d0b20 in read_event (argv=0x7ffed792f4a8) at ../src/nvim/event/rstream.c:190 0x00000000004ce462 in multiqueue_process_events (this=0x7f96e18172d0) at ../src/nvim/event/multiqueue.c:150 0x000000000059b630 in nv_event (cap=0x7ffed792f620) at ../src/nvim/normal.c:7908 0x000000000058be69 in normal_execute (state=0x7ffed792f580, key=-25341) at ../src/nvim/normal.c:1137 0x0000000000652463 in state_enter (s=0x7ffed792f580) at ../src/nvim/state.c:61 0x000000000058a1fe in normal_enter (cmdwin=false, noexmode=false) at ../src/nvim/normal.c:467 0x00000000005500c2 in main (argc=2, argv=0x7ffed792f8d8) at ../src/nvim/main.c:554 Alternative approach suggested by bfredl is to use close_cb of the process. My unsuccessful attempt is below. (It seems close_cb is queued too late, which is the similar problem addressed by this commit): commit 75fc12c6ab15711bdb7b18c6d42ec9d157f5145e Author: Justin M. Keyes Date: Fri Aug 18 01:30:41 2017 +0200 rpc: use Stream's close_cb instead of explicit check in receive_msgpack() diff --git a/src/nvim/event/process.c b/src/nvim/event/process.c index 8371d3cd482e..e52da23cdc40 100644 --- a/src/nvim/event/process.c +++ b/src/nvim/event/process.c @@ -416,6 +416,10 @@ static void on_process_exit(Process *proc) static void on_process_stream_close(Stream *stream, void *data) { Process *proc = data; + ILOG("on_process_stream_close"); + if (proc->stream_close_cb != NULL) { + proc->stream_close_cb(stream, proc->stream_close_data); + } decref(proc); } diff --git a/src/nvim/event/process.h b/src/nvim/event/process.h index 5c00e8e7ecd5..34a8d54f6f8c 100644 --- a/src/nvim/event/process.h +++ b/src/nvim/event/process.h @@ -26,6 +26,11 @@ struct process { Stream *in, *out, *err; process_exit_cb cb; internal_process_cb internal_exit_cb, internal_close_cb; + + // Called when any of the process streams (in/out/err) closes. + stream_close_cb stream_close_cb; + void *stream_close_data; + bool closed, detach; MultiQueue *events; }; @@ -50,6 +55,8 @@ static inline Process process_init(Loop *loop, ProcessType type, void *data) .closed = false, .internal_close_cb = NULL, .internal_exit_cb = NULL, + .stream_close_cb = NULL, + .stream_close_data = NULL, .detach = false }; } diff --git a/src/nvim/event/stream.c b/src/nvim/event/stream.c index 7c865bfe1e8c..c8720d1e45d9 100644 --- a/src/nvim/event/stream.c +++ b/src/nvim/event/stream.c @@ -95,7 +95,11 @@ void stream_close(Stream *stream, stream_close_cb on_stream_close, void *data) void stream_close_handle(Stream *stream) FUNC_ATTR_NONNULL_ALL { + ILOG("stream=%d", stream); + // LOG_CALLSTACK(); if (stream->uvstream) { + // problem: this schedules on the queue, but channel.c:receive_msgpack may + // be processed before close_cb is called by libuv. uv_close((uv_handle_t *)stream->uvstream, close_cb); } else { uv_close((uv_handle_t *)&stream->uv.idle, close_cb); @@ -105,6 +109,7 @@ void stream_close_handle(Stream *stream) static void close_cb(uv_handle_t *handle) { Stream *stream = handle->data; + ILOG(">>>>>>>>>>>>>>>>>>>>>>> stream=%p stream->internal_close_cb=%p", stream, stream->internal_close_cb); if (stream->buffer) { rbuffer_free(stream->buffer); } diff --git a/src/nvim/msgpack_rpc/channel.c b/src/nvim/msgpack_rpc/channel.c index 782eabe04e4a..dc2b794e366a 100644 --- a/src/nvim/msgpack_rpc/channel.c +++ b/src/nvim/msgpack_rpc/channel.c @@ -128,6 +128,8 @@ uint64_t channel_from_process(Process *proc, uint64_t id, char *source) source); incref(channel); // process channels are only closed by the exit_cb channel->data.proc = proc; + channel->data.proc->stream_close_cb = close_cb2; + channel->data.proc->stream_close_data = channel; wstream_init(proc->in, 0); rstream_init(proc->out, 0); @@ -387,17 +389,6 @@ static void receive_msgpack(Stream *stream, RBuffer *rbuf, size_t c, goto end; } - if ((chan_wstream(channel) != NULL && chan_wstream(channel)->closed) - || (chan_rstream(channel) != NULL && chan_rstream(channel)->closed)) { - char buf[256]; - snprintf(buf, sizeof(buf), - "ch %" PRIu64 ": stream closed unexpectedly. " - "closing channel", - channel->id); - call_set_error(channel, buf, WARN_LOG_LEVEL); - goto end; - } - size_t count = rbuffer_size(rbuf); DLOG("ch %" PRIu64 ": parsing %u bytes from msgpack Stream: %p", channel->id, count, stream); @@ -571,23 +562,6 @@ static Stream *chan_wstream(Channel *chan) abort(); } -/// Returns the Stream that a Channel reads from. -static Stream *chan_rstream(Channel *chan) -{ - switch (chan->type) { - case kChannelTypeSocket: - return &chan->data.stream; - case kChannelTypeProc: - return chan->data.proc->out; - case kChannelTypeStdio: - return &chan->data.std.in; - case kChannelTypeInternal: - return NULL; - } - abort(); -} - - static bool channel_write(Channel *channel, WBuffer *buffer) { bool success = false; @@ -799,6 +773,12 @@ static void close_cb(Stream *stream, void *data) decref(data); } +static void close_cb2(Stream *stream, void *data) +{ + ILOG("close_cb2"); + close_channel(data); +} + /// @param source description of source function, rplugin name, TCP addr, etc static Channel *register_channel(ChannelType type, uint64_t id, MultiQueue *events, char *source) --- test/functional/api/server_requests_spec.lua | 30 +++++++++++++++++++++------- 1 file changed, 23 insertions(+), 7 deletions(-) (limited to 'test/functional/api/server_requests_spec.lua') diff --git a/test/functional/api/server_requests_spec.lua b/test/functional/api/server_requests_spec.lua index 6a32f979ea..9f245d913b 100644 --- a/test/functional/api/server_requests_spec.lua +++ b/test/functional/api/server_requests_spec.lua @@ -20,6 +20,22 @@ describe('server -> client', function() cid = nvim('get_api_info')[1] end) + it('handles unexpected closed stream while preparing RPC response', function() + source([[ + let g:_nvim_args = [v:progpath, '--embed', '-n', '-u', 'NONE', '-i', 'NONE', ] + let ch1 = jobstart(g:_nvim_args, {'rpc': v:true}) + let child1_ch = rpcrequest(ch1, "nvim_get_api_info")[0] + call rpcnotify(ch1, 'nvim_eval', 'rpcrequest('.child1_ch.', "nvim_get_api_info")') + + let ch2 = jobstart(g:_nvim_args, {'rpc': v:true}) + let child2_ch = rpcrequest(ch2, "nvim_get_api_info")[0] + call rpcnotify(ch2, 'nvim_eval', 'rpcrequest('.child2_ch.', "nvim_get_api_info")') + + call jobstop(ch1) + ]]) + eq(2, eval("1+1")) -- Still alive? + end) + describe('simple call', function() it('works', function() local function on_setup() @@ -141,7 +157,7 @@ describe('server -> client', function() end) end) - describe('when the client is a recursive vim instance', function() + describe('recursive (child) nvim client', function() if os.getenv("TRAVIS") and helpers.os_name() == "osx" then -- XXX: Hangs Travis macOS since e9061117a5b8f195c3f26a5cb94e18ddd7752d86. pending("[Hangs on Travis macOS. #5002]", function() end) @@ -155,7 +171,7 @@ describe('server -> client', function() after_each(function() command('call rpcstop(vim)') end) - it('can send/recieve notifications and make requests', function() + it('can send/receive notifications and make requests', function() nvim('command', "call rpcnotify(vim, 'vim_set_current_line', 'SOME TEXT')") -- Wait for the notification to complete. @@ -188,7 +204,7 @@ describe('server -> client', function() end) end) - describe('when using jobstart', function() + describe('jobstart()', function() local jobid before_each(function() local channel = nvim('get_api_info')[1] @@ -227,7 +243,7 @@ describe('server -> client', function() end) end) - describe('when connecting to another nvim instance', function() + describe('connecting to another (peer) nvim', function() local function connect_test(server, mode, address) local serverpid = funcs.getpid() local client = spawn(nvim_argv) @@ -256,7 +272,7 @@ describe('server -> client', function() client:close() end - it('over a named pipe', function() + it('via named pipe', function() local server = spawn(nvim_argv) set_session(server) local address = funcs.serverlist()[1] @@ -265,7 +281,7 @@ describe('server -> client', function() connect_test(server, 'pipe', address) end) - it('to an ip adress', function() + it('via ip address', function() local server = spawn(nvim_argv) set_session(server) local address = funcs.serverstart("127.0.0.1:") @@ -273,7 +289,7 @@ describe('server -> client', function() connect_test(server, 'tcp', address) end) - it('to a hostname', function() + it('via hostname', function() local server = spawn(nvim_argv) set_session(server) local address = funcs.serverstart("localhost:") -- cgit From 2a3bcd1ff883429a3dd17e7ae5ddc1396abbad17 Mon Sep 17 00:00:00 2001 From: Björn Linse Date: Sun, 29 Oct 2017 03:06:53 +0100 Subject: rpc: Don't delay notifications when request is pending (#6544) With the old behavior, if a GUI makes a blocking request that requires user interaction (like nvim_input()), it would not get any screen updates. The client, not nvim, should decide how to handle notifications during a pending request. If an rplugin wants to avoid async calls while a sync call is busy, it likely wants to avoid processing async calls while another async call also is handled as well. This may break the expectation of some existing rplugins. For compatibility, remote/define.vim reimplements the old behavior. Clients can opt-out by specifying `sync=urgent`. - Legacy hosts should be updated to use `sync=urgent`. They could add a flag indicating which async methods are always safe to call and which must wait until the main loop returns. - New hosts can expose the full asyncness, they don't need to offer both behaviors. ref #6532 ref #1398 d83868fe9071af1b4866594eac12f7aa0fa71b53 --- test/functional/api/server_requests_spec.lua | 61 +++++++++++++++++++--------- 1 file changed, 42 insertions(+), 19 deletions(-) (limited to 'test/functional/api/server_requests_spec.lua') diff --git a/test/functional/api/server_requests_spec.lua b/test/functional/api/server_requests_spec.lua index 9f245d913b..4380e52b8b 100644 --- a/test/functional/api/server_requests_spec.lua +++ b/test/functional/api/server_requests_spec.lua @@ -109,7 +109,28 @@ describe('server -> client', function() end) describe('requests and notifications interleaved', function() - -- This tests that the following scenario won't happen: + it('does not delay notifications during pending request', function() + local received = false + local function on_setup() + eq("retval", funcs.rpcrequest(cid, "doit")) + stop() + end + local function on_request(method) + if method == "doit" then + funcs.rpcnotify(cid, "headsup") + eq(true,received) + return "retval" + end + end + local function on_notification(method) + if method == "headsup" then + received = true + end + end + run(on_request, on_notification, on_setup) + end) + + -- This tests the following scenario: -- -- server->client [request ] (1) -- client->server [request ] (2) triggered by (1) @@ -124,36 +145,38 @@ describe('server -> client', function() -- only deals with one server->client request at a time. (In other words, -- the client cannot send a response to a request that is not at the top -- of nvim's request stack). - -- - -- But above scenario shoudn't happen by the way notifications are dealt in - -- Nvim: they are only sent after there are no pending server->client - -- request(the request stack fully unwinds). So (3) is only sent after the - -- client returns (6). - it('works', function() - local expected = 300 - local notified = 0 + pending('will close connection if not properly synchronized', function() local function on_setup() eq('notified!', eval('rpcrequest('..cid..', "notify")')) end local function on_request(method) - eq('notify', method) - eq(1, eval('rpcnotify('..cid..', "notification")')) - return 'notified!' + if method == "notify" then + eq(1, eval('rpcnotify('..cid..', "notification")')) + return 'notified!' + elseif method == "nested" then + -- do some busywork, so the first request will return + -- before this one + for _ = 1, 5 do + eq(2, eval("1+1")) + end + eq(1, eval('rpcnotify('..cid..', "nested_done")')) + return 'done!' + end end local function on_notification(method) - eq('notification', method) - if notified == expected then - stop() - return + if method == "notification" then + eq('done!', eval('rpcrequest('..cid..', "nested")')) + elseif method == "nested_done" then + -- this should never have been sent + ok(false) end - notified = notified + 1 - eq('notified!', eval('rpcrequest('..cid..', "notify")')) end run(on_request, on_notification, on_setup) - eq(expected, notified) + -- ignore disconnect failure, otherwise detected by after_each + clear() end) end) -- cgit From a39c8b7ce30ddeed4329c28c42b1b699103dccab Mon Sep 17 00:00:00 2001 From: James McCoy Date: Thu, 2 Nov 2017 05:45:38 -0400 Subject: test: server_spec: Tolerate missing protocol (#7478) Travis disabled IPv6: [ RUN ] serverstart(), serverstop() parses endpoints correctly: FAIL ...build/neovim/neovim/test/functional/eval/server_spec.lua:83: Expected objects to be the same. Passed in: (table) { [1] = '127.0.0.1:12345' } Expected: (table) { [1] = '127.0.0.1:12345' *[2] = '::1:12345' } Change all tests to ensure a server was actually started before expecting it to be returned from serverlist(). --- test/functional/api/server_requests_spec.lua | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) (limited to 'test/functional/api/server_requests_spec.lua') diff --git a/test/functional/api/server_requests_spec.lua b/test/functional/api/server_requests_spec.lua index 4380e52b8b..a2a198ca83 100644 --- a/test/functional/api/server_requests_spec.lua +++ b/test/functional/api/server_requests_spec.lua @@ -304,14 +304,30 @@ describe('server -> client', function() connect_test(server, 'pipe', address) end) - it('via ip address', function() + it('via ipv4 address', function() local server = spawn(nvim_argv) set_session(server) local address = funcs.serverstart("127.0.0.1:") + if #address == 0 then + pending('no ipv4 stack', function() end) + return + end eq('127.0.0.1:', string.sub(address,1,10)) connect_test(server, 'tcp', address) end) + it('via ipv6 address', function() + local server = spawn(nvim_argv) + set_session(server) + local address = funcs.serverstart('::1:') + if #address == 0 then + pending('no ipv6 stack', function() end) + return + end + eq('::1:', string.sub(address,1,4)) + connect_test(server, 'tcp', address) + end) + it('via hostname', function() local server = spawn(nvim_argv) set_session(server) -- cgit From 91b856cccec89d5a29c280441f4a6aa7c8393268 Mon Sep 17 00:00:00 2001 From: Björn Linse Date: Sat, 10 Jun 2017 15:25:23 +0200 Subject: channels: tests --- test/functional/api/server_requests_spec.lua | 1 + 1 file changed, 1 insertion(+) (limited to 'test/functional/api/server_requests_spec.lua') diff --git a/test/functional/api/server_requests_spec.lua b/test/functional/api/server_requests_spec.lua index a2a198ca83..37ac532d18 100644 --- a/test/functional/api/server_requests_spec.lua +++ b/test/functional/api/server_requests_spec.lua @@ -262,6 +262,7 @@ describe('server -> client', function() eq("done!",funcs.rpcrequest(jobid, "write_stderr", "fluff\n")) eq({'notification', 'stderr', {0, {'fluff', ''}}}, next_message()) funcs.rpcrequest(jobid, "exit") + eq({'notification', 'stderr', {0, {''}}}, next_message()) eq({'notification', 'exit', {0, 0}}, next_message()) end) end) -- cgit From fd4021387e696c7a2da4ad776789cfbb938d5332 Mon Sep 17 00:00:00 2001 From: "Justin M. Keyes" Date: Fri, 9 Mar 2018 00:24:38 +0100 Subject: test: rename next_message() to next_msg() --- test/functional/api/server_requests_spec.lua | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) (limited to 'test/functional/api/server_requests_spec.lua') diff --git a/test/functional/api/server_requests_spec.lua b/test/functional/api/server_requests_spec.lua index 37ac532d18..18229b54ff 100644 --- a/test/functional/api/server_requests_spec.lua +++ b/test/functional/api/server_requests_spec.lua @@ -6,7 +6,7 @@ local Paths = require('test.config.paths') local clear, nvim, eval = helpers.clear, helpers.nvim, helpers.eval local eq, neq, run, stop = helpers.eq, helpers.neq, helpers.run, helpers.stop local nvim_prog, command, funcs = helpers.nvim_prog, helpers.command, helpers.funcs -local source, next_message = helpers.source, helpers.next_message +local source, next_msg = helpers.source, helpers.next_msg local ok = helpers.ok local meths = helpers.meths local spawn, nvim_argv = helpers.spawn, helpers.nvim_argv @@ -258,12 +258,12 @@ describe('server -> client', function() it('rpc and text stderr can be combined', function() eq("ok",funcs.rpcrequest(jobid, "poll")) funcs.rpcnotify(jobid, "ping") - eq({'notification', 'pong', {}}, next_message()) + eq({'notification', 'pong', {}}, next_msg()) eq("done!",funcs.rpcrequest(jobid, "write_stderr", "fluff\n")) - eq({'notification', 'stderr', {0, {'fluff', ''}}}, next_message()) + eq({'notification', 'stderr', {0, {'fluff', ''}}}, next_msg()) funcs.rpcrequest(jobid, "exit") - eq({'notification', 'stderr', {0, {''}}}, next_message()) - eq({'notification', 'exit', {0, 0}}, next_message()) + eq({'notification', 'stderr', {0, {''}}}, next_msg()) + eq({'notification', 'exit', {0, 0}}, next_msg()) end) end) -- cgit