| Commit message (Collapse) | Author | Age |
| |
|
| |
|
|
|
|
|
|
|
| |
We already have an extensive suite of static analysis tools we use,
which causes a fair bit of redundancy as we get duplicate warnings. PVS
is also prone to give false warnings which creates a lot of work to
identify and disable.
|
|
|
|
|
|
| |
Problem: The style guide states that all switch statements that are not conditional on an enum must have a `default` case, but does not give any explicit guideline for switch statements that are conditional on enums. As a result, a `default` case is added in many enum switch statements, even when the switch statement is exhaustive. This is not ideal because it removes the ability to have compiler errors to easily detect unchanged switch statements when a new possible value for an enum is added.
Solution: Add explicit guidelines for switch statements that are conditional on an enum, clarifying that a `default` case is not necessary if the switch statement is exhaustive. Also refactor pre-existing code with unnecessary `default` cases.
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
If nvim exited with nonzero status this is for one of the two reasons
- `:cquit` was invoked. This is used by users and plugins to communicate
a result, like a nonzero status will fail a `git commit` operation
- There was an internal error or deadly signal. in this case an error
message was likely written to stderr or to the screen.
In the latter case, the error message was often hidden by the TUI
exiting altscreen mode, which erases all visible terminal text.
This change prevents this in the latter case, while still cleaning up
the terminal properly when `:cquit` was deliberatily invoked.
Other cleanup like exiting mouse mode and raw mode is still done.
|
|
|
| |
fix(startup): run embedded process with real path
|
|
|
|
| |
BREAKING CHANGE: Unsaved changes are now preserved rather than discarded
when stdio channel is closed.
|
|
|
| |
Also add the EXITFREE definition to main_lib rather than the nvim target, as the header generation needs the EXITFREE flag to work properly.
|
|
|
|
|
| |
Rename stdin/stdout in the server, so that RPC data won't get corrupted.
This also restores the use of stderr to write directly to the terminal.
|
| |
|
| |
|
|
|
|
|
|
|
|
|
|
| |
Allow Include What You Use to remove unnecessary includes and only
include what is necessary. This helps with reducing compilation times
and makes it easier to visualise which dependencies are actually
required.
Work on https://github.com/neovim/neovim/issues/549, but doesn't close
it since this only works fully for .c files and not headers.
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
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.
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Problem:
1. Log messages (especially in CI) are hard to correlate with tests.
2. Since b353a5c05f02 #11886, dumplog() prints the logs next to test
failures. This is noisy and gets in the way of the test results.
Solution:
1. Associate an incrementing id with each test and include it in log
messages.
- FUTURE: add v:name so Nvim instances can be formally "named"?
2. Mention "child" in log messages if the current Nvim is a child (based
on the presence of $NVIM).
BEFORE:
DBG … 12345 UI: event
DBG … 12345 log_server_msg:722: RPC ->ch 1: …
DBG … 12345 UI: flush
DBG … 12345 inbuf_poll:444: blocking... events_enabled=1 events_pending=0
DBG … 23454 UI: stop
INF … 23454 os_exit:594: Nvim exit: 0
AFTER:
DBG … T57 UI: event
DBG … T57 log_server_msg:722: RPC ->ch 1: …
DBG … T57 UI: flush
DBG … T57 inbuf_poll:444: blocking... events_enabled=1 events_pending=0
DBG … T57/child UI: stop
INF … T57/child os_exit:594: Nvim exit: 0
|
| |
|
| |
|
|
|
| |
ref #15440
|
|
|
|
|
|
|
|
|
|
| |
After a process's refcnt is decremented to zero, it enqueues a
`process_close_event` on its own event queue. In `process_wait`, this
event should be processed immediately so that any process close
callbacks are executed before `process_wait` returns.
Update `process_wait` to always process the process's event queue after
the process is freed, rather than the event queue passed in as an
argument.
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Before #12937, the only way to specify the `$TERM` for a pty job was
through the `TERM` key in the job's opts dict. This was shuttled to the
child process throug a special field on the PtyProcess object and
injected into the environment after forking.
Now that we have a proper way to specify the environment for a job, we
can simply ensure that the env dict has a proper `TERM` set and avoid
the extra shuttling of data around.
This deprecates the use of the `TERM` option, but will still honor it if
present, although at a lower priority than a `TERM` present in the env
dict.
This also fixes #13874 because we're no longer trying to overwrite
`TERM` in the env dict with the special pty `term_name`. Doing so
raises an internal error because of the existing key which, under
certain circumstances, would cause the "hit enter" prompt. However,
since the child process had already forked, there was no way for the
user to acknowledge the prompt and we would just hang there.
|
| |
|
|
|
|
|
| |
Uses `128 + term_signal` in case of exit due to a signal.
Fixes https://github.com/neovim/neovim/issues/10571.
|
|
|
|
|
|
|
|
|
|
| |
- process_stop: do not close stdin explicitly. The "close stdin" step was from
aa9cb48, before we fixed/reworked the SIGTERM timing logic. So it's probably
outdated / no longer needed.
- win: jobstop: exit_code 15
GetExitCodeProcess appears to return the used signal.
https://docs.microsoft.com/en-us/windows/win32/api/processthreadsapi/nf-processthreadsapi-getexitcodeprocess
ref #10573
|
|
|
|
|
|
|
|
|
| |
- Move __gcov_flush to process_spawn, for more reliable coverage
tracking of subprocesses
- Travis: use GCOV_ERROR_FILE
- codecov: use "-X fix" to skip "fixing" uploaded coverage data; it
should be handled by codecov's backend instead.
- AppVeyor: no $PATH mangling, which breaks with the improved coverage tracking
due to missing .dll in PATH.
|
|
|
|
|
|
| |
This restores missing coverage again.
Move it to process_spawn in os/pty_process_unix.c, since it seems to
break printargs-test on Windows/AppVeyor otherwise (#10248).
|
|
|
| |
Fixes https://github.com/neovim/neovim/pull/3926#issuecomment-502343527.
|
|\
| |
| | |
closes #4983
|
| |
| |
| |
| | |
Not familiar with the code, but I assume that loop_poll_events can actually
change stream->num_bytes, so condition is not always false.
|
| |
| |
| |
| |
| |
| |
| | |
shada_write_file() is called on exit (:quit and friends), this can be
very slow.
Note: AFAICT Vim (do_viminfo()) does not appear to fsync() viminfo.
|
| |
| |
| |
| |
| |
| |
| |
| |
| | |
1. Don't check elapsed time in children_kill_cb(), it's already implied
by the start-time of the timer itself.
2. Restart timer from children_kill_cb() for PTY jobs, to send SIGKILL
after SIGTERM. There is an edge case where SIGKILL might follow
SIGTERM too quickly, if jobstop() is called near the 2-second timer
window. But this edge case is not worth code complication.
|
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
Before f31c26f1afb5 the timer was used to try SIGTERM *and* SIGKILL, so
a repeating timer was needed. After f31c26f1afb5 process_stop() sends
SIGTERM immediately, and the timer only sends SIGKILL.
So we don't need a repeating timer.
- Simplifies the logic: don't need to call uv_timer_stop() explicitly.
- Avoids a problem: if process_stop() is called more than once in the
2-second window, the first on_process_exit() would call
uv_timer_stop() which stops the timer for all stopped processes.
|
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
children_kill_cb() is racey. One obvious problem is that
process_close_handles() is *queued* by on_process_exit(), so when
children_kill_cb() is invoked, the dead process might still be in the
`loop->children` list. If the OS already reclaimed the dead PID, Nvim
may try to SIGKILL it.
Avoid that by checking `proc->status`.
Vim doesn't have this problem because it doesn't attempt to kill
processes that ignored SIGTERM after a timeout.
closes #8269
|
|/
|
|
|
| |
It serves no purpose because process_stop() is already guarded by
`proc->stopped_time`.
|
|
|
|
|
|
|
|
| |
XXX: comment at https://stackoverflow.com/q/1173342 :
> Windows recycles PIDs quite fast, you have to be extra careful not
> to kill unrelated processes. These APIs will report PPIDs for long
> dead processes whose PIDs may have been recycled. Check the parent
> start date to make sure it is related to the processes you spawned.
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
UV_PROCESS_DETACHED compels libuv:uv__process_child_init() to call
setsid() in the child just after fork(). That ensures the process and
its descendants are grouped in a separate session (and process group).
The following jobstart() call correctly groups `sh` and `sleep` in a new
session (and process-group), where `sh` is the "session leader" (and
process-group leader):
:call jobstart(['sh','-c','sleep 60'])
SESN PGRP PID PPID Command
30383 30383 30383 3620 │ ├─ -bash
30383 31432 31432 30383 │ │ └─ nvim -u NORC
30383 31432 31433 30383 │ │ ├─ nvim -u NORC
8105 8105 8105 31432 │ │ └─ sh -c sleep 60
8105 8105 8106 8105 │ │ └─ sleep 60
closes #6530
ref: https://stackoverflow.com/q/1046933
ref: https://unix.stackexchange.com/a/404065
Helped-by: Marco Hinz <mh.codebro+github@gmail.com>
Discussion
------------------------------------------------------------------------
On my linux box before this patch, the termclose_spec.lua:'kills job
trapping SIGTERM' test indirectly causes cmake/busted to wait for 60s.
That's because the test spawns a `sleep 60` descendant process which
hangs around even after nvim exits: nvim killed the parent PID, but not
PGID (process-group), so the grandchild "reparented" to init (PID 1).
Session contains processes (and process-groups) which are logically part
of the same "login session". Process-group is a set of
logically/informally-related processes within a session; for example,
shells assign a process group to each "job". Session IDs and PGIDs both
have type pid_t (like PIDs).
These OS-level mechanisms are, as usual, legacy accidents whose purpose
is upheld by convention and folklore. We can use session-level grouping
(setsid), or we could use process-group-level grouping (setpgid).
Vim uses setsid() if available, otherwise setpgid(0,0).
Windows
------------------------------------------------------------------------
UV_PROCESS_DETACHED on win32 sets CREATE_NEW_PROCESS_GROUP flag.
But uv_kill() does not kill the process-group:
https://github.com/nodejs/node/issues/3617
Ideas:
- Set UV_PROCESS_DETACHED (CREATE_NEW_PROCESS_GROUP), then call
GenerateConsoleCtrlEvent(CTRL_BREAK_EVENT, pid)
- Maybe won't work because MSDN says "Only processes that share the
same console as the calling process receive the signal."
https://docs.microsoft.com/en-us/windows/console/generateconsolectrlevent
But CREATE_NEW_PROCESS_GROUP creates a new console ...
ref https://stackoverflow.com/q/1453520
- Group processes within a "job". libuv does that *globally* for
non-detached processes: uv__init_global_job_handle.
- Iterate through CreateToolhelp32Snapshot().
- https://stackoverflow.com/q/1173342
- Vim does this, see terminate_all()
|
| |
|
| |
|
| |
|
| |
|
| |
|
|
|
|
|
|
|
| |
Doc for UV_PROCESS_DETACHED in uv.h mentions:
> child process will still keep the parent's event loop alive unless
> the parent process calls uv_unref() on the child's process handle.
ref #3944
|
| |
|
|
|
|
|
| |
This reverts the revert of #6644 (7c1a5d1d4), and handles it properly
now (with tests).
|
| |
|
|
|
|
| |
This reverts commit 34c3f03013375817d3d089e685793290eded553a.
|
|
|
|
|
|
|
| |
Send SIGTERM to processes directly, instead of waiting for ~1s.
- removes TERM_TIMEOUT
- changes KILL_TIMEOUT to milliseconds
- removes Process.term_sent
|
| |
|
| |
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
`lib/queue.h` implements a basic queue. `event/queue.c` implements
a specialized data structure on top of lib/queue.h; it is not a "normal"
queue.
Rename the specialized multi-level queue implemented in event/queue.c to
"multiqueue", to avoid confusion when reading the code.
Before this change one can eventually notice that "macros (uppercase
symbols) are for the normal queue, lowercase operations are for the
multi-level queue", but that is unnecessary friction for new developers
(or existing developers just visiting this part of the codebase).
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Inherited signal mask may block SIGCHLD, which causes libuv to hang at
epoll_wait.
Closes #5230
Helped-by: Nicolas Hillegeer <nicolas@hillegeer.com>
Helped-by: John Szakmeister <john@szakmeister.net>
Note: the #pragma gymnastics are a workaround for broken system headers on
macOS.
signal.h:
int sigaddset(sigset_t *, int);
#define sigaddset(set, signo) (*(set) |= __sigbits(signo), 0)
sys/_types/_sigset.h:
typedef __darwin_sigset_t sigset_t;
sys/_types.h:
typedef __uint32_t __darwin_sigset_t; /* [???] signal set */
sigset_t is defined as unsigned int, but the sigaddset() ORs it with an int,
mixing the types. So GCC generates a sign-conversion warning:
sig.c:9:13: warning: implicit conversion changes signedness: 'int' to 'unsigned int' [-Wsign-conversion]
(*(&s) |= __sigbits((sigset_t) 20), 0);
~~ ^~~~~~~~~~~~~~~~~~~~~~~~
1 warning generated.
System headers are normally ignored when the compiler generates warnings:
https://gcc.gnu.org/onlinedocs/cpp/System-Headers.html
> GCC gives code found in system headers special treatment. All warnings,
> other than those generated by ‘#warning’ (see Diagnostics), are suppressed
> while GCC is processing a system header. Macros defined in a system header
> are immune to a few warnings wherever they are expanded. This immunity is
> granted on an ad-hoc basis, when we find that a warning generates lots of
> false positives because of code in macros defined in system headers.
Instead of the #pragma workaround, we could cast the sigset_t pointer:
# if defined(__APPLE__)
sigaddset((int *)&mask, SIGCHLD);
# else
sigaddset(&mask, SIGCHLD);
# endif
but that could break if the headers are later fixed.
|