| Commit message (Collapse) | Author | Age |
|
|
|
|
|
| |
Make the copy_object() family accept an optional arena. More than
half of the callsites should be refactored to use an arena later
anyway.
|
|
|
|
|
| |
This is both simpler in client code and more effective (always reuse
block hottest in cache)
|
| |
|
| |
|
|
|
|
|
| |
This reduces the memory overhead for large redraw batches, as a much smaller
prefix of the api object buffer is used and needs to be hot in cache.
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
In the rare case that exit_event is called from process_close_handles,
it stalls waiting for the process to exit (the routine is currently
underway to do just that). This causes `job_spec.lua` to sometimes
stall.
REJECTED IDEAS:
==============================================================
1. Currently `exit_event` is placed on `main_loop.fast_events`. Would the problem
be solved by using `main_loop.events` instead?
- A: Maybe, but it will cause other problems, such as queuing exit_event()
during "Press Enter..." prompt which may result in the event not being
processed, leading to another stall.
2. Can we avoid the timer?
- A: Using a timer is just the easiest way to queue a delayed event without
causing an infinite loop in the queue currently being processed.
3. Can we avoid the new `exit_need_delay` global...
1. by using `process_is_tearing_down` instead?
- A: Can't use `process_is_tearing_down` because its semantics are different.
2. by checking a similar condition as `process_teardown`? https://github.com/neovim/neovim/blob/f50135a32e11c535e1dc3a8e9460c5b4e640ee86/src/nvim/event/process.c#L141-L142
```
if (!process_is_tearing_down || (kl_empty(main_loop.children) && multiqueue_empty(main_loop.events))) {
uv_timer_start(&main_loop.exit_delay_timer, exit_delay_cb, 0, 0);
return;
}
```
- A: Tried but it did not work (other stalls occurred). Maybe
exit_event() is called from a source other than
process_close_handles() and is delayed, the delayed exit_event() will
be executed before main_loop.events is processed, resulting in an
infinite loop.
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Note for external UIs: Nvim can now emit multiple "redraw" event batches
before a final "flush" event is received. To retain existing behavior,
clients should make sure to update visible state at an explicit "flush"
event, not just the end of a "redraw" batch of event.
* Get rid of copy_object() blizzard in the auto-generated ui_event layer
* Special case "grid_line" by encoding screen state directly to
msgpack events with no intermediate API events.
* Get rid of the arcane notion of referring to the screen as the "shell"
* Array and Dictionary are kvec_t:s, so define them as such.
* Allow kvec_t:s, such as Arrays and Dictionaries, to be allocated with
a predetermined size within an arena.
* Eliminate redundant capacity checking when filling such kvec_t:s
with values.
|
|
|
|
|
|
|
|
|
| |
drawback: tracing memory errors with ASAN is less accurate for arena
allocated memory.
Therefore, to start with it is being used for Object types around
serialization/deserialization exclusively. This is going to have
a large impact especially when TUI is refactored as a co-prosess
as all UI events will be serialized and deserialized by nvim itself.
|
|
|
|
|
|
|
|
|
|
| |
Currently this is more or less a straight off reimplementation,
but this allow further optimizations down the line, especially
for avoiding memory allocations of rpc objects.
Current score for "make functionaltest; make oldtest" on a -DEXITFREE build:
is 117 055 352 xfree(ptr != NULL) calls (that's NUMBERWANG!).
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Problem:
1. The main log routine does not protect itself against recursion.
log_lock() doesn't guard against recursion, it would deadlock...
2. 22b52dd462e5 (#11501) regressed 6f27f5ef91b3 (#10172), because
set_init_1..process_spawn tries to log (see backtrace below), but the
mutex isn't initialized yet. Even if the mutex were valid, we don't
want early logging to fallback to stderr because that can break
embedders when stdio is used for RPC.
frame 1: 0x00000001001d54f4 nvim`open_log_file at log.c:205:7
frame 2: 0x00000001001d5390 nvim`logmsg(log_level=1, context="UI: ", func_name=0x0000000000000000, line_num=-1, eol=true, fmt="win_viewport") at log.c:150:20
frame : 0x000000010039aea2 nvim`ui_call_win_viewport(grid=2, win=1000, topline=0, botline=1, curline=0, curcol=0, line_count=1) at ui_events_call.generated.h:321:3
frame 4: 0x00000001003dfefc nvim`ui_ext_win_viewport(wp=0x0000000101816400) at window.c:939:5
frame 5: 0x00000001003ec5b4 nvim`win_ui_flush at window.c:7303:7
frame 6: 0x00000001003a04c0 nvim`ui_flush at ui.c:508:3
frame 7: 0x00000001002966ba nvim`do_os_system(argv=0x0000600000c0c000, input=0x0000000000000000, len=0, output=0x0000000000000000, nread=0x00007ff7bfefe830, silent=false, forward_output=false) at shell.c:894:3
frame 8: 0x0000000100295f68 nvim`os_call_shell(cmd="unset nonomatch; vimglob() { while [ $# -ge 1 ]; do echo \"$1\"; shift; done }; vimglob >/var/folders/gk/3tttv_md06987tlwpyp62jrw0000gn/T/nvimwwvwfD/0 ~foo", opts=kShellOptExpand | kShellOptSilent | kShellOptHideMess, extra_args=0x0000000000000000) at shell.c:663:18
frame 9: 0x0000000100295845 nvim`call_shell(cmd="unset nonomatch; vimglob() { while [ $# -ge 1 ]; do echo \"$1\"; shift; done }; vimglob >/var/folders/gk/3tttv_md06987tlwpyp62jrw0000gn/T/nvimwwvwfD/0 ~foo", opts=kShellOptExpand | kShellOptSilent | kShellOptHideMess, extra_shell_arg=0x0000000000000000) at shell.c:712:14
frame 10: 0x0000000100294c6f nvim`os_expand_wildcards(num_pat=1, pat=0x00007ff7bfefeb20, num_file=0x00007ff7bfefee58, file=0x00007ff7bfefee60, flags=43) at shell.c:328:7
...
frame 23: 0x000000010028ccef nvim`expand_env_esc(srcp=",~foo", dst="~foo", dstlen=4094, esc=false, one=false, prefix=0x0000000000000000) at env.c:673:17
frame 24: 0x000000010026fdd5 nvim`option_expand(opt_idx=29, val=",~foo") at option.c:1950:3
frame 25: 0x000000010026f129 nvim`set_init_1(clean_arg=false) at option.c:558:19
frame 26: 0x00000001001ea25e nvim`early_init(paramp=0x00007ff7bfeff5f0) at main.c:198:3
frame 27: 0x00000001001ea6bf nvim`main(argc=1, argv=0x00007ff7bfeff848) at main.c:255:3
Solution:
1. Check for recursion, show "internal error" message.
- FUTURE: when "remote TUI" is merged, can we remove log_lock()?
2. Skip logging if log_init wasn't called yet.
|
| |
|
| |
|
|
|
|
| |
co-authored-by: hlpr98 <hlpr98@gmail.com>
|
|
|
|
|
| |
Possibly dialog code is messages.c could be moved here as well.
misc1.c is now empty, so delete it.
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
* refactor: format with uncrustify
* fixup(dundar): fix functions comments
* fixup(dundar): remove space between variable and ++/--
* fixup(dundar): better workaround for macro attributes
This is done to be able to better use uncrustify rules for macros
* fixup(justin): make preprocessors follow neovim style guide
|
|
|
|
|
|
| |
* refactor: disable formatting for attribute in macro
* fixup: disable/enable uncrustify with uncrustify:indent-off/on
* fixup: stop indenting contents inside braces in case
* fixup: remove case brace if no variable declaration
|
|
|
|
|
|
|
|
|
|
| |
Note: the reason for removing them is not that there after this refactor
is no use of them, but rather that having them available is an
anti-pattern: they manange an _extra_ heap allocation which has
nothing to do with the functionality of the map itself (khash
manages the real buffers internally). In case there happens to
be a reason to allocate the map structure itself later, this
should be made explicit using xcalloc/xfree calls.
|
|
|
|
| |
[skip ci]
|
| |
|
| |
|
|
|
|
|
| |
- No code changes
- Move it to main.c
|
|
|
|
|
| |
"Multicast" is perhaps a more conventional name for the concept.
"One-shot" is the conventional name for how the event is (currently)
scheduled.
|
|
|
|
|
|
| |
This makes external UI behave consistenly with TUI w.r.t resizes.
Which will be needed anyway as TUI will use the external UI protocol
soon.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
../src/nvim/event/rstream.c:119:44: warning: format specifies type 'void *' but the argument has type 'Stream *' (aka 'struct stream *') [-Wformat-pedantic]
DLOG("Closing Stream (%p): %s (%s)", stream,
~~ ^~~~~~
../src/nvim/event/stream.c:95:30: warning: format specifies type 'void *' but the argument has type 'Stream *' (aka 'struct stream *') [-Wformat-pedantic]
DLOG("closing Stream: %p", stream);
~~ ^~~~~~
../src/nvim/msgpack_rpc/channel.c:71:72: warning: format specifies type 'void *' but the argument has type 'Stream *' (aka 'struct stream *') [-Wformat-pedantic]
DLOG("rpc ch %" PRIu64 " in-stream=%p out-stream=%p", channel->id, in, out);
~~ ^~
../src/nvim/msgpack_rpc/channel.c:71:76: warning: format specifies type 'void *' but the argument has type 'Stream *' (aka 'struct stream *') [-Wformat-pedantic]
DLOG("rpc ch %" PRIu64 " in-stream=%p out-stream=%p", channel->id, in, out);
~~ ^~~
../src/nvim/msgpack_rpc/channel.c:226:28: warning: format specifies type 'void *' but the argument has type 'Stream *' (aka 'struct stream *') [-Wformat-pedantic]
channel->id, count, stream);
^~~~~~
|
| |
|
|
|
|
|
| |
We already do this for _invalid_ async requests #9300.
Now we also do it for failed invocation of valid requests.
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
According to [MessagePack RPC specification](https://github.com/msgpack-rpc/msgpack-rpc),
message ID must be 32-bit unsigned integer. But Neovim implementation
uses uint64_t instead of uint32_t. This can have wrong results in the
case of large ids or a malformed request, for example:
Actual response: [1,18446744073709551615,[1,"Message is not an array"],null]
Expected response: [1,4294967295,[1,"Message is not an array"],null]
The issue does not affect RPC clients written in dynamically-typed
languages like Python. Wrong type of sequence id number breaks RPC
clients written statically typed languages like C/C++/Golang: all of
them expect uint32_t as message id.
Examples:
https://github.com/msgpack-rpc/msgpack-rpc-cpp/blob/11268ba2be5954ddbb2b7676c7da576985e45cfc/src/msgpack/rpc/protocol.h#L27
https://github.com/ugorji/go/blob/master/codec/msgpack.go#L993
closes #8850
|
|
|
|
|
|
|
|
|
|
| |
Using a sentinel value in the response-id is ambiguous because the
msgpack-rpc spec allows all values (including zero/max). And clients
control the id, so we can't be sure they won't use the sentinel value.
Instead of a sentinel value, check the message type explicitly.
ref #8850
|
|
|
|
|
|
|
|
|
|
| |
Note about shada.c:
- shada_read_next_item_start was intentionally shadowing `unpacked` and
`i` because many of the macros (e.g. ADDITIONAL_KEY) implicitly
depended on those variable names.
- Macros were changed to parameterize `unpacked` (but not `i`). Macros
like CLEAR_GA_AND_ERROR_OUT do control-flow (goto), so any other
approach is messy.
|
|
|
|
|
| |
Previously, nvim sent a response with invalid request id (UINT64_MAX).
In functionaltests, catch unexpected error notifications in after_each().
|
| |
|
| |
|
|
|
|
|
|
| |
Give embeders a chance to set up nvim, by processing a request before
startup. This allows an external UI to show messages and prompts from
--cmd and buffer loading (e.g. swap files)
|
|
|
|
| |
close #8745
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
channel.c: WIP remove redundant method check and added FUNC_ATTR_NONNULL_ALL macro
channel.c channel_defs.h helpers.c: added Error field to RequestEvent, added no_op handler func
channel.c: use const char* instead of string and cleanup
channel.c; channel_defs.h; helpers.c: removed error from event again; send errors directly to the channel without using handlers and events
channel.c: fixed memory leak and lint errors
api/private/dispatch.c; api/vim.c; msgpack_rpc/channel.c msgpack_rpc/helpers.c added Error* field to msgpack_get_handler_for; further refactored channel.c
channel.c:323 changed order of evaluation in if statement
channel.c: removed superflous whitespace
dispatch.c: review comment
|
| |
|
|
|
|
|
| |
Fire autocmd when channel opens or its info changes.
Add a way for API clients can describe themselves.
|
| |
|
| |
|
| |
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
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
|
| |
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
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)
|
| |
|
| |
|
| |
|
|
|
|
|
|
| |
- Establish ERROR log level as "critical". Such errors are rare and will
be valuable when users encounter unusual circumstances.
- Set -DMIN_LOG_LEVEL=3 for release-type builds
|