Commit Graph

1791 Commits

Author SHA1 Message Date
Drew Neil
642e14d9e7 Repair ex_terminal_spec functional tests 2017-08-21 20:44:03 +01: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
Justin M. Keyes
88165a798e clipboard: test g:clipboard validation, fix a bug
Also fix `:help foo` highlighting in health.vim
2017-08-20 22:17:03 +02:00
Justin M. Keyes
cc7e344f83 clipboard: remove start_batch_changes() in redir_write()
start_batch_changes() doesn't avoid invoking the clipboard
once-per-line, because the loop is actually in ex_echo(), which calls
redir_write() for each message. But we've already entered
start_batch_changes() by then, so that was never the problem.

    redir_write at /home/vagrant/old.neovim/build/../src/nvim/message.c:2523
    msg_puts_attr_len at /home/vagrant/old.neovim/build/../src/nvim/message.c:1600
    msg_outtrans_len_attr at /home/vagrant/old.neovim/build/../src/nvim/message.c:1221
    ex_echo at /home/vagrant/old.neovim/build/../src/nvim/eval.c:19433
    do_one_cmd at /home/vagrant/old.neovim/build/../src/nvim/ex_docmd.c:2242

Trying to defer _explicit_ clipboard updates is difficult.
    :redir @+ | silent echo system('cat foo') | redir END
is essentially equivalent to:
    for l in readfile('foo')
        let @+ .= l
    endfor
We cannot make judgements about when to ignore a script's bad decisions.
start_batch_changes() only works around the case of clipboard=unnamed,
i.e. _implicit_ clipboard updates (`:g/foo/d`).  Not explicit
assignment.
2017-08-20 20:01:22 +02:00
Justin M. Keyes
9882e25dc4 clipboard: avoid error flood during :redir
redir_write():
- This is a "batch" operation which was not yet covered by
  start_batch_changes()

adjust_clipboard_name():
- msg() and friends during :redir will, of course, cause redir_write()
  to try to capture that message, which causes recursion.
- EMSG() here is trouble: if it interrupts :redir it is a mess.
  Rather than deal with the mess, show a non-error message.

closes #7182
closes #7184
closes #7183
ref #6048
ref #7032
2017-08-20 19:49:42 +02:00
Justin M. Keyes
9a6eb71eba test/win: give up on this one 2017-08-16 09:13:45 +02:00
Justin M. Keyes
dbb404542b test/win: place cursor at edge to tickle SIGWINCH 2017-08-16 09:13:44 +02:00
Justin M. Keyes
91c85a6378 test: tty-test.c: keep tty_out handle around
Now the window_split_tab_spec.lua test seems to work.
Also do some cleanup.
2017-08-16 09:13:44 +02:00
Justin M. Keyes
d2d76882f7 win/test: enable more :terminal tests
To deal with SIGWINCH limitations on Windows, change some resize tests
to _shrink_ the screen width. ... But this didn't work, so still
ignoring those tests on Windows.
2017-08-16 09:13:44 +02:00
Justin M. Keyes
e0763e94ad test: tty-test.c: restore win32 SIGWINCH handler 2017-08-16 09:13:44 +02:00
Justin M. Keyes
6a90f53862 test: cleanup 2017-08-16 09:13:44 +02:00
erw7
d3a8c4f992 win/pty: log errors 2017-08-16 09:13:44 +02:00
erw7
1614e805b3 win/test: tty-test: print screen size explicitly with CTRL-Q
tty-test.exe causes abnormal termination with low repeatability, try
changing it so as not to use SIGWINCH.
2017-08-16 09:13:44 +02:00
erw7
4b1f21de75 win: support :terminal 2017-08-16 09:13:43 +02:00
Justin M. Keyes
bb70eec177 Merge #6364 'command-line color hook' 2017-08-16 00:20:37 +02:00
ZyX
19a28352a9 ex_getln: Make error messages look better 2017-08-14 01:56:48 +03:00
ZyX
0571b8cb0e functests: Alter comment 2017-08-14 01:22:10 +03:00
ZyX
a5449f79ac functests: Check that input is correctly silenced 2017-08-14 01:17:16 +03:00
Nikolai Aleksandrovich Pavlov
bf1b1ea6ee lua/executor: Fix crash when printing empty string (#7157) 2017-08-13 17:37:35 +02:00
James McCoy
ac055d677a os_stat: return ENOENT on NULL filename arg
Closes #4370

Explication:

    In the backtrace in #4370, we see that `buf_write()` was called with
    non-NULL `fname` and `sfname` arguments, but they've since _become_
    NULL.

    #7  0x00000000004de09d in buf_write (buf=0x1dee040, fname=0x0, fname@entry=0x1e985b0 "/home/sean/src/github.com/snczl/virta/pkg/meld/segment.go",
                                         sfname=0x0, sfname@entry=0x1ddfa60 "segment.go", start=1, end=72, eap=eap@entry=0x7ffc6b032e60, append=0,
                                         forceit=0, reset_changed=1, filtering=0)
    at /home/travis/build/neovim/bot-ci/build/neovim/src/nvim/fileio.c:2576

    This is most likely due to the code that restores those values from
    `buf`, which happens just before the fatal call to `os_fileinfo`

    ```c
        /*
         * The autocommands may have changed the name of the buffer, which may
         * be kept in fname, ffname and sfname.
         */
        if (buf_ffname)
          ffname = buf->b_ffname;
        if (buf_sfname)
          sfname = buf->b_sfname;
        if (buf_fname_f)
          fname = buf->b_ffname;
        if (buf_fname_s)
          fname = buf->b_sfname;
    ```

    It's worth noting that at this point `ffname` is still non-NULL, so
    it _could_ be used.  However, our current code is purely more strict
    than Vim in this area, which has caused us problems before (e.g.,
    `getdigits()`).  The commentary for `struct file_buffer` clearly
    indicate that all of `b_ffname`, `b_sfname`, and `b_fname` may be
    NULL:

    ```c
      /*
       * b_ffname has the full path of the file (NULL for no name).
       * b_sfname is the name as the user typed it (or NULL).
       * b_fname is the same as b_sfname, unless ":cd" has been done,
       *		then it is the same as b_ffname (NULL for no name).
       */
      char_u      *b_ffname;        /* full path file name */
      char_u      *b_sfname;        /* short file name */
      char_u      *b_fname;         /* current file name */
    ```

    Vim directly calls `stat(2)` which, although it is annotated to tell
    the compiler that the path argument is non-NULL, does handle a NULL
    pointer by returning a `-1` value and setting `errno` to `EFAULT`.
    This satisfies Vim's check, since it treats any `-1` return from
    `stat(2)` to mean the file doesn't exist (at least in this code
    path).

    Note that Vim's mch_stat() implementations on win32 and solaris
    clearly cannot accept NULL `name`. But the codepaths that call
    mch_stat will NULL `name` tend to be unix-only (eg: u_read_undo)!
2017-08-10 00:56:07 +02:00
ZyX
474aa823dc Merge branch 'master' into colored-cmdline 2017-08-06 15:25:17 +03:00
ZyX
efb03903eb functests: Remove wait() from input_spec 2017-08-06 14:43:46 +03:00
Justin M. Keyes
3827d5bc71 input: skip dialogs if no UI is active
Treat dialogs in the same way as "silent mode" (`nvim -es`).

References #1984
References #3901
2017-08-06 04:22:45 +02:00
Justin M. Keyes
30996359ef clint: allow starting brace after enum 2017-08-06 02:51:49 +02:00
Justin M. Keyes
efb0aca0ee test/helpers: disable powershell logo/banner message 2017-08-06 02:51:48 +02:00
Justin M. Keyes
a31482db4d terminal: block redraw during c_CTRL-D
Unlike the normal wildmenu, the CTRL-D wild-list is not restored by
statusline redraw. (Semantics: ^D is controlled by 'wildoptions' option,
so it's in the "wild..." family.)

TODO: externalize the c_CTRL-D wild-list.
2017-08-05 21:32:34 +02:00
Justin M. Keyes
c695443727 win_redr_status(): skip if wildmenu is showing
This might be too coarse, but it passes all tests ...

A more nuanced approach might be: only skip the windows whose
statuslines are overwritten by the wildmenu.

Closes #2255
Closes #7108

vim-patch:8.0.0710 N/A because of the changes in this commit.
2017-08-05 21:32:34 +02:00
Justin M. Keyes
92101947fe test: job_spec.lua: disable shada in test instance 2017-08-04 22:25:39 +02:00
ZyX
fbe60af538 Merge branch 'master' into colored-cmdline 2017-07-31 02:05:02 +03:00
ckelsel
5cd68b3900 vim-patch:8.0.0124 #7092
Problem:    Internal error for assert_inrange(1, 1).
Solution:   Adjust number of allowed arguments. (Dominique Pelle)

3421566376
2017-07-30 14:15:26 +02:00
Justin M. Keyes
3b45f676c0 menu_get(): doc 2017-07-28 02:34:24 +02:00
Matthieu Coudron
dc685387a3 viml: introduce menu_get() function #6322
menu_get({path}, {modes}). See :h menu_get.
2017-07-28 01:27:58 +02:00
ZyX
1011462b40 Revert "functests: Replace wait() with nvim_async"
This reverts commit e129607988.

Tests stopped working in CI.
2017-07-27 18:49:13 +03:00
ZyX
c5857e3f38 ex_getln: Cache highlight callback calling results 2017-07-26 22:56:48 +03:00
ZyX
e129607988 functests: Replace wait() with nvim_async 2017-07-26 22:04:39 +03:00
ZyX
1ba21b4a31 functests: Remove unneeded wait()s 2017-07-26 13:02:45 +03:00
ZyX
0a46ae3c0a functests: Add sleep to <C-c> test 2017-07-18 01:29:41 +03:00
ZyX
25f669049c functests: Test input() nesting support 2017-07-18 01:17:59 +03:00
ZyX
759f71d50e functests: Check for previously unchecked errors 2017-07-18 00:34:39 +03:00
ZyX
8a581b918b ex_getln: Check prev_prompt_errors before running redrawcmdline
Otherwise there will be infinite recursion and shortly a crash. Running 
redrawcmdline recursively occurs under color_cmdline_error label.
2017-07-18 00:20:21 +03:00
ZyX
cfb1d937a6 api helpers: Also save and restore did_emsg 2017-07-18 00:08:57 +03:00
ZyX
f4744e1821 ex_getln: Do not goto color_cmdline_end without first cleaning up
The issue with debug mode was actually not cleaning up after `try_enter`: 
location `&tstate` was pointing to got invalidated and received some “garbage” 
(actually, values that got stored on the stack afterwards). But pointer to that 
garbage was still stored in `msg_list`, so next attempt to check it resulted in 
a crash.
2017-07-17 01:55:10 +03:00
Justin M. Keyes
d72db1ac8f test: mouse_spec: remove obnoxious wait times (#7029)
helpers.skip_fragile() already skips the problematic tests
on the ASan build. But the 15s timeout plus 5s 'mousetime'
cause the tests to take 1+ minutes anyways.
2017-07-15 18:00:00 +02:00
ZyX
69719e658c Merge branch 'master' into colored-cmdline 2017-07-15 18:56:45 +03:00
Justin M. Keyes
d15317ece4 health.vim: fix test 2017-07-15 16:15:47 +02:00
James McCoy
d67e48379f test: shada: Replace hard-coded 704 with dynamic version 2017-07-14 06:44:22 -04:00
Nikolai Aleksandrovich Pavlov
702c94aacf mbyte: Fix crash when using multibyte chars in maparg() return (#6986)
This is a refactoring typo from #6947.

Fixes #6985

Combined with #6947 where typo was made it also fixes vim/vim#1827 which was
present in Neovim.
2017-07-09 10:40:46 +02:00
KillTheMule
619838f85d inccommand: improve performance #6949
During a preview, we can stop looking for matches after we got enough
lines for the preview buffer.

Because of this perf improvement, the 'redrawtime' test needs to be
slowed down in a different way: _long_ lines instead of just many lines.
2017-07-08 17:51:42 +02:00
Justin M. Keyes
0ea7e45bc1 'cpoptions': remove "<" flag; ignore <special>
Closes #6937 "nvim_get_keymap output is unreliable"
2017-07-08 16:34:35 +02:00
d10n
69e9cda5ac i_CTRL-O: fix :startinsert at end of line (#6963)
The gchar_cursor() == NUL check is already done in ins_ctrl_o.
ins_esc changes gchar_cursor() so this if block is probably never
entered.

Issue:

Pressing CTRL-O in insert mode at the end of the line and typing
:startinsert moves the cursor 1 column back, when I expect the cursor
to remain at the end of the line

This is a regression from Vim behavior. Since at least Vim version 7.0,
Vim returns you to insert mode at the end of the line.

091e7d033c is the first bad neovim commit

Steps to reproduce using `nvim -u NORC`:
`aaaa<C-o>:startinsert<CR>`

Fixes #6962
2017-07-08 12:50:58 +02:00