Commit Graph

41 Commits

Author SHA1 Message Date
Justin M. Keyes
3abf17ae88 API: validation: mention invalid method name (#8489) 2018-06-07 10:56:44 +02:00
Justin M. Keyes
0f442c328e channel.c:call_set_error(): fix memory leak 2017-08-21 01:04:28 +02:00
Justin M. Keyes
af993da435 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=<optimized out>, 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 <justinkz@gmail.com>
    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)
2017-08-21 01:04:28 +02:00
ZyX
09f849b600 Merge branch 'master' into luaviml'/lua 2017-05-08 15:43:45 +03:00
Justin M. Keyes
3ea1007753 api: nvim_get_mode()
Asynchronous API functions are served immediately, which means pending
input could change the state of Nvim shortly after an async API function
result is returned.

nvim_get_mode() is different:
  - If RPCs are known to be blocked, it responds immediately (without
    flushing the input/event queue)
  - else it is handled just-in-time before waiting for input, after
    pending input was processed. This makes the result more reliable
    (but not perfect).

Internally this is handled as a special case, but _semantically_ nothing
has changed: API users never know when input flushes, so this internal
special-case doesn't violate that. As far as API users are concerned,
nvim_get_mode() is just another asynchronous API function.

In all cases nvim_get_mode() never blocks for more than the time it
takes to flush the input/event queue (~µs).

Note: This doesn't address #6166; nvim_get_mode() will provoke #6166 if
e.g. `d` is operator-pending.

Closes #6159
2017-04-28 19:14:34 +02:00
Justin M. Keyes
086c354a0a api: Do not translate error messages.
Also re-word some error messages:
- "Key does not exist: %s"
- "Invalid channel: %<PRIu64>"
- "Request array size must be 4 (request) or 3 (notification)"
- "String cannot contain newlines"

References #6150
2017-04-24 00:11:27 +02:00
Justin M. Keyes
3fbc660d57 api_set_error(): rename 2017-04-23 23:44:57 +02:00
Justin M. Keyes
2ed91f222f api/internal: Remove set field from Error type. 2017-04-23 23:44:32 +02:00
Sander Bosma
5c9860a0a2 api: Do not truncate errors <1 MB. #6237
Closes #5984
2017-04-23 23:44:29 +02:00
ZyX
c2f3e361c5 *: Add comment to all C files 2017-04-19 19:11:50 +03:00
ZyX
043d8ff9f2 Merge branch 'master' into luaviml'/lua 2017-04-08 01:54:58 +03:00
James McCoy
f4a3a96b68 Add handling for MSGPACK_OBJECT_FLOAT{32,64}
msgpack-c previously only had MSGPACK_OBJECT_FLOAT, which was a 64-bit
value.  Now, 32-bit and 64-bit floats are supported as distinct types,
but we'll simply continue to treat everything as 64-bit types.
2017-03-30 21:35:36 -04:00
ZyX
28dafe3ff0 eval,*: Move get_tv_string to typval.c
Function was renamed and changed to return `const char *`.
2017-03-29 10:08:05 +03:00
ZyX
62fde31936 api: Also shift numbers in api_metadata output
Fixes problem introduced by “api: Allow kObjectTypeNil to be zero without 
breaking compatibility”: apparently there are clients which use metadata and 
there are which aren’t. For the first that commit would not be needed, for the 
second that commit misses this critical piece.
2017-03-27 00:12:23 +03:00
ZyX
ae4adcc707 gendeclarations: Make declarations generator work with macros funcs
Now it checks functions also after every semicolon and closing figure brace, 
possibly preceded by whitespaces (tabs and spaces). This should make messing 
with declarations in macros not needed.
2017-03-27 00:12:23 +03:00
ZyX
22d3ce9c29 msgpack_rpc: Fix #HANDLE_TYPE_CONVERSION_IMPL
Function declarations generator is able to handle properly only the *first* 
function definition that is in macros, and only if it is the first entity in the 
macros. So msgpack_rpc_from_* was already really a static function, additionally 
its attributes were useless. This commit switches to explicit declarations and 
makes generated functions static.
2017-03-27 00:12:23 +03:00
ZyX
ca4c8b7f8a api: Allow kObjectTypeNil to be zero without breaking compatibility 2017-03-27 00:12:23 +03:00
ZyX
ba2f615cd4 functests: Test for error conditions
During testing found the following bugs:

1. msgpack-gen.lua script is completely unprepared for Float values either in 
   return type or in arguments. Specifically:

   1. At the time of writing relevant code FLOAT_OBJ did not exist as well as 
      FLOATING_OBJ, but it would be used by msgpack-gen.lua should return type 
      be Float. I added FLOATING_OBJ macros later because did not know that 
      msgpack-gen.lua uses these _OBJ macros, otherwise it would be FLOAT_OBJ.
   2. msgpack-gen.lua should use .data.floating in place of .data.float. But it 
      did not expect that .data subattribute may have name different from 
      lowercased type name.

2. vim_replace_termcodes returned its argument as-is if it receives an empty 
   string (as well as _vim_id*() functions did). But if something in returned 
   argument lives in an allocated memory such action will cause double free: 
   once when freeing arguments, then when freeing return value. It did not cause 
   problems yet because msgpack bindings return empty string as {NULL, 0} and 
   nothing was actually allocated.
3. New code in msgpack-gen.lua popped arguments in reversed order, making lua 
   bindings’ signatures be different from API ones.
2017-03-27 00:11:28 +03:00
James McCoy
b37d062459 msgpack_rpc: Allocate empty string in msgpack_rpc_to_object
STR_CASE previously used a NULL data pointer for the String object, but
this pushes the NULL checks to the rest of the code.  Instead,
allocating an empty string solves the same issue of there not being any
data but ensures that we're not passing NULL to functions that don't
expect it.

Closes #5627
2016-11-29 08:19:48 -05:00
James McCoy
ca292c9768 Avoid serializing NULL string through msgpack
Attempting to serialize a NULL string through msgpack results in
msgpack_sbuffer_write attempting to memcpy from a NULL pointer, which is
undefined behavior.
2016-11-23 20:30:00 -05:00
James McCoy
654e92186b lint 2016-11-02 10:06:27 -04:00
James McCoy
37e64d79cc object_to_vim: Fix buffer/window/tabpage conversion on BE systems
Since data.integer is a different (larger) integer type than
data.{buffer,window,tabpage}, we cannot abuse the union by using
data.integer to access the value for all 4 types.  Instead, remove the
{buffer,window,tabpage} members and always use the integer member.

In order to accomodate this, perform distinct validation and coercion
between the Integer type and Buffer/Window/Tabpage types in
object_to_vim, msgpack_rpc helpers, and gendispatch.lua.
2016-11-02 10:06:27 -04:00
Björn Linse
f6968dc0f7 api: call multiple methods atomically (useful in async contexts)
remove unused response_id parameter of handle_nvim_... helpers
2016-10-22 10:51:32 +02:00
Björn Linse
a2d25b7bf8 api: unify buffer numbers and window ids with handles
also allow handle==0 meaning curbuf/curwin/curtab
2016-08-31 21:40:20 +02:00
Björn Linse
de3a515123 api: rename "msgpack_rpc/defs.h" to "api/private/dispatch.h" and use the header generator. 2016-08-31 21:40:20 +02:00
ZyX
458a4d0444 *: Fix linter errors
Also adds one exception to linter rules:

    typedef struct {
      kvec_t(Object) stack;
    } EncodedData;

is completely valid (from the style guide point of view) code.
2016-06-24 17:16:11 +03:00
ZyX
90b8cf133e msgpack_rpc: Also make msgpack_from_*/msgpack_to_* functions not recur
This removes some stack overflows in new test regarding deeply nested variables.
Now in place of crashing vim_to_object/msgpack_rpc_from_object/etc it crashes
clear_tv with stack overflow.
2016-06-24 16:53:26 +03:00
ZyX
d359bb3f60 *: Fix errors from new linter checks 2016-06-11 00:08:57 +03:00
ZyX
cf4e1fb0f4 *: Fix new linter errors
Originally there were 128 new errors, so I thought this is a good idea to fix 
all of them. Of course, this commit also fixes many suppressed errors.
2016-05-01 20:35:51 +03:00
oni-link
8373aaf44e helpers.c: Handle msgpack str/bin objects with length 0 correctly
When converting a msgpack object to a String object, strings (and byte
arrays) with length 0 are handled as errors. This is fixed by
always using the msgpack data pointer as a valid pointer. For a NULL
pointer there is nothing to copy.

Test by @snoe

Fixes #3844
2015-12-21 00:23:53 +01:00
Björn Linse
57d3a2a52f api: represent api type String as msgpack type STR. closes #1250 2015-10-08 20:18:46 +02:00
Florian Walch
bfadf5a28b ASan: Fix "null pointer passed for argument declared to never be null". #2925
Arguments passed to xmemdupz() are sometimes NULL, but xmemdupz() has
FUNC_ATTR_NONNULL_ALL. Check pointers for NULL before calling
xmemdupz().

Resolves #2533.
2015-06-30 18:36:41 +03:00
Björn Linse
5bbd182a3e msgpack-rpc: handle failure to convert method arguments #2664 2015-05-19 16:21:01 +02:00
Scott Prager
676133aa9b msgpack: Allow notifications to execute commands.
Consider: `let vim = rpcstart('nvim', ['--embed'])`

Allows `rpcnotify(vim, ...)` to work like an asynchronous
`rpcrequest(nvim, ...)`.

Helped-by: Michael Reed <m.reed@mykolab.com>
Helped-by: Justin M. Keyes <>
2015-04-13 10:20:42 -04:00
Thiago de Arruda
e0e41b30c6 ui: Remove/adapt some old code for a big UI refactor
- Remove abstract_ui global, now it is always active
- Remove some terminal handling code
- Remove unused functions
- Remove HAVE_TGETENT/TERMINFO/TERMIOS/IOCTL #ifdefs
- Remove tgetent/terminfo from version.c
- Remove curses/terminfo dependencies
- Only start/stop termcap when starting/exiting the program
- msg_use_printf will return true if there are no attached UIs(
  messages will be written to stdout)
- Remove `ex_winpos`(implement `:winpos` with `ex_ni`)
2015-02-16 23:17:39 -03:00
Michael Reed
ec5e94518e build: upgrade msgpack-c #1900
closes #1899
closes #1967

refs https://github.com/msgpack/msgpack-c/pull/194
2015-02-09 23:51:02 -05:00
Rui Abreu Ferreira
da3f097807 msgpack-rpc: Fix typo in validation message 2014-11-07 13:43:01 -03:00
Rui Abreu Ferreira
214d54b42e msgpack-rpc: Return from msgpack_rpc_validate on error
- When validating a msgpack msg we need to return on the first error
  otherwise we can SEGFAULT with invalid checks
2014-11-07 13:42:43 -03:00
Thiago de Arruda
64844b7312 msgpack-rpc: Improve logging of msgpack-rpc messages
- Expose more logging control from the log.c module(get log stream and omit
  newlines)
- Remove logging from the generated functions in msgpack-gen.lua
- Refactor channel.c/helpers.c to log every msgpack-rpc payload using
  msgpack_object_print(a helper function from msgpack.h)
- Remove the api_stringify function, it was only useful for logging msgpack-rpc
  which is now handled by msgpack_object_print.
2014-10-23 21:46:03 -03:00
Thiago de Arruda
72e3e57bf1 msgpack-rpc: Allow selective deferral API calls
Since all API functions now run immediately after a msgpack-rpc request is
parsed by libuv callbacks, a mechanism was added to override this behavior and
allow certain functions to run in Nvim main loop.

The mechanism is simple: Any API function tagged with the FUNC_ATTR_DEFERRED (a
"dummy" attribute only used by msgpack-gen.lua) will be called when Nvim main
loop receives a K_EVENT key.

To implement this mechanism it was necessary some restructuration on the
msgpack-rpc modules, especially in the msgpack-gen.lua script.
2014-10-21 11:05:49 -03:00
Thiago de Arruda
b280308ac6 msgpack-rpc: Create subdirectory for msgpack-rpc modules
Create the msgpack_rpc subdirectory and move all modules that deal with
msgpack-rpc to it. Also merge msgpack_rpc.c into msgpack_rpc/helpers.c
2014-10-21 11:05:49 -03:00