aboutsummaryrefslogtreecommitdiff
Commit message (Collapse)AuthorAge
...
| * | rpc: close channel if stream was closedJustin M. Keyes2017-08-21
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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)
| * | log: some DEBUG-level stream loggingJustin M. Keyes2017-08-21
| | |
* | | Merge pull request #7192 from llorens/vim-8.0.0092James McCoy2017-08-25
|\ \ \ | | | | | | | | vim-patch:8.0.0092
| * | | vim-patch:8.0.0092Lech Lorens2017-08-21
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Problem: C indenting does not support nested namespaces that C++ 17 has. Solution: Add check that passes double colon inside a name. (Pauli, closes vim/vim#1214) https://github.com/vim/vim/commit/ca8b8d6956dd881de6446fc32c38e817a364a6cc
* | | | Merge #7205 from justinmk/win-wmainJustin M. Keyes2017-08-24
|\ \ \ \
| * | | | win: expect utf8-encoded `argv` when built as a libraryJustin M. Keyes2017-08-24
| | | | |
| * | | | win: wmain(): use utf16_to_utf8() #7060Justin M. Keyes2017-08-24
| | | | |
* | | | | tui: always use unibi_add_ext_str with unibi_get_ext_str #7204Fredrik Fornwall2017-08-24
|/ / / / | | | | | | | | | | | | | | | | | | | | | | | | | | | | When using an index returned by unibi_add_ext_str() we should always use unibi_get_ext_str() and not rely on the index being lower than unibi_string_begin_. Closes #7206
* | | | win: wmain(): locale-independent argv (#7180)Yuto Tokunaga2017-08-23
| | | | | | | | | | | | fix #7060
* | | | clipboard: disallow recursion; show hint only once (#7203)Justin M. Keyes2017-08-22
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | - Show hint only once per session. - provider#clipboard#Call(): prevent recursion - provider#clear_stderr(): use has_key(), because :silent! is still captured by :redir. closes #7184
* | | | Merge #6973 from teto/normal_hlJustin M. Keyes2017-08-22
|\ \ \ \
| * | | | syntax.c: styleMatthieu Coudron2017-08-22
| | | | | | | | | | | | | | | | | | | | | | | | | Converts some documentation to doxygen format + minor styling improvements.
| * | | | syntax.c: register 'Normal' highlight groupMatthieu Coudron2017-08-22
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | - :hi Normal works with -u NONE - Makes HL_TABLE and ATTR_ENTYRY a function instead of a macro so that in can be used in gdb. - Introduces ATTRENTRY_INIT to init attrentry_t
* | | | | Merge pull request #7201 from jamessan/vim-8.0.0308James McCoy2017-08-22
|\ \ \ \ \ | |/ / / / |/| | | | vim-patch:8.0.0308,8.0.0325,8.0.0326,8.0.0437,8.0.0612,8.0.0680
| * | | | vim-patch:8.0.0680James McCoy2017-08-21
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Problem: Plugins in start packages are sourced twice. (mseplowitz) Solution: Use the unmodified runtime path when loading plugins (test by Ingo Karkat, closes vim/vim#1801) https://github.com/vim/vim/commit/07ecfa64a18609a986f21d6132d04ee8934f3200
| * | | | vim-patch:8.0.0612James McCoy2017-08-21
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Problem: Package directories are added to 'runtimepath' only after loading non-package plugins. Solution: Split off the code to add package directories to 'runtimepath'. (Ingo Karkat, closes vim/vim#1680) https://github.com/vim/vim/commit/ce876aaa9a250a5a0d0e34b3a2625e51cf9bf5bb
| * | | | vim-patch:8.0.0437James McCoy2017-08-21
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Problem: The packadd test does not create the symlink correctly and does not test the right thing. Solution: Create the directory and symlink correctly. https://github.com/vim/vim/commit/644df41c44cbdfacdedbba55ef77a6c6031eccd8
| * | | | vim-patch:8.0.0326James McCoy2017-08-21
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Problem: Packadd test uses wrong directory name. Solution: Use the variable name value. (Hirohito Higashi) https://github.com/vim/vim/commit/24f8f543d4036c5d2ce4ea6973a174cf2176cb72
| * | | | vim-patch:8.0.0325James McCoy2017-08-21
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Problem: Packadd test does not clean up symlink. Solution: Delete the link. (Hirohito Higashi) https://github.com/vim/vim/commit/913727e56761d57aaba61197c2d3485418dea7eb
| * | | | vim-patch:8.0.0308James McCoy2017-08-21
|/ / / / | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Problem: When using a symbolic link, the package path will not be inserted at the right position in 'runtimepath'. (Dugan Chen, Norio Takagi) Solution: Resolve symbolic links when finding the right position in 'runtimepath'. (Hirohito Higashi) https://github.com/vim/vim/commit/2f9e575583c2ad3978ee3d0f790eeff7df56bd6c
* | | | Merge pull request #6808 from nelstrom/normal-mode-terminalJames McCoy2017-08-21
|\ \ \ \ | |_|/ / |/| | | Make :terminal remain in normal mode when created
| * | | Repair tui_spec functional testsDrew Neil2017-08-21
| | | |
| * | | Repair job_spec functional testsDrew Neil2017-08-21
| | | |
| * | | Repair ex_terminal_spec functional testsDrew Neil2017-08-21
| | | |
| * | | Update documentationDrew Neil2017-08-21
| | | |
| * | | Use Normal mode as default when opening a new terminalDrew Neil2017-08-21
|/ / /
* | | Merge #7193 from justinmk/cb-pathologyJustin M. Keyes2017-08-21
|\ \ \ | |/ / |/| |
| * | clipboard: test g:clipboard validation, fix a bugJustin M. Keyes2017-08-20
| | | | | | | | | | | | Also fix `:help foo` highlighting in health.vim
| * | clipboard: remove start_batch_changes() in redir_write()Justin M. Keyes2017-08-20
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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.
| * | clipboard: avoid error flood during :redirJustin M. Keyes2017-08-20
|/ / | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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
* | Merge #7171 from justinmk/docJustin M. Keyes2017-08-19
|\ \
| * | doc/api: nvim_out_write() and friendsJustin M. Keyes2017-08-18
| | | | | | | | | | | | References #7178
| * | version: tweak layout, docJustin M. Keyes2017-08-18
| | |
| * | intro: remove byline #6984Justin M. Keyes2017-08-18
| | |
| * | nvim -h: omit special-case optionsJustin M. Keyes2017-08-18
| | | | | | | | | | | | | | | | | | Group some options, and sort them alphabetically. `nvim -h` should fit on one (smallish) screen. Uncommon options don't need to be here, they live in the :help.
| * | docJustin M. Keyes2017-08-18
|/ /
* | Merge pull request #7185 from jamessan/fix-provider-clear_stderrJames McCoy2017-08-18
|\ \ | | | | | | provider#clear_stderr: Use remove() not delete() to update s:stderr
| * | provider#clear_stderr: Use remove() not delete() to update s:stderrJames McCoy2017-08-18
|/ / | | | | | | Ref #7184
* | Merge #7007 'Windows :terminal'Justin M. Keyes2017-08-16
|\ \ | | | | | | | | | References #6383 Closes #4748
| * | test/win: give up on this oneJustin M. Keyes2017-08-16
| | |
| * | test/win: place cursor at edge to tickle SIGWINCHJustin M. Keyes2017-08-16
| | |
| * | test: tty-test.c: keep `tty_out` handle aroundJustin M. Keyes2017-08-16
| | | | | | | | | | | | | | | Now the window_split_tab_spec.lua test seems to work. Also do some cleanup.
| * | win/test: enable more :terminal testsJustin M. Keyes2017-08-16
| | | | | | | | | | | | | | | | | | 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.
| * | test: tty-test.c: restore win32 SIGWINCH handlerJustin M. Keyes2017-08-16
| | |
| * | test: cleanupJustin M. Keyes2017-08-16
| | |
| * | single-includes: ignore os/pty_process_win.hJustin M. Keyes2017-08-16
| | |
| * | pty_process_win: avoid quoting for cmd.exeerw72017-08-16
| | |
| * | win/pty: log errorserw72017-08-16
| | |
| * | win/pyt: cleanuperw72017-08-16
| | |
| * | win/test: tty-test: print screen size explicitly with CTRL-Qerw72017-08-16
| | | | | | | | | | | | | | | tty-test.exe causes abnormal termination with low repeatability, try changing it so as not to use SIGWINCH.
generated by cgit (git 2.34.1) at 2025-07-16 09:24:41 +0000