feat(server): set $NVIM, unset $NVIM_LISTEN_ADDRESS #11009

PROBLEM
------------------------------------------------------------------------
$NVIM_LISTEN_ADDRESS has conflicting purposes as both a parameter ("the
current process should listen on this address") and a descriptor ("the
current process is a child of this address").

This contradiction means the presence of NVIM_LISTEN_ADDRESS is
ambiguous, so child Nvim always tries to listen on its _parent's_
socket. This is the cause of lots of  "Failed to start server" spam in
our test/CI logs:

    WARN  2022-04-30… server_start:154: Failed to start server: address already in use: \\.\pipe\nvim-4480-0
    WARN  2022-04-30… server_start:154: Failed to start server: address already in use: \\.\pipe\nvim-2168-0

SOLUTION
------------------------------------------------------------------------

1. Set $NVIM to the parent v:servername, *only* in child processes.
   - Now the correct way to detect a "parent" Nvim is to check for $NVIM.
2. Do NOT set $NVIM_LISTEN_ADDRESS in child processes.
3. On startup if $NVIM_LISTEN_ADDRESS exists, unset it immediately after
   server init.
4. Open a channel to parent automatically, expose it as v:parent.

Fixes #3118
Fixes #6764
Fixes #9336
Ref https://github.com/neovim/neovim/pull/8247#issuecomment-380275696
Ref #8696

src/nvim/eval.h

@@ -164,7 +164,7 @@ typedef enum {
   VV_ARGV,
   VV_COLLATE,
   VV_EXITING,
-  // Neovim
+  // Nvim
   VV_STDERR,
   VV_MSGPACK_TYPES,
   VV__NULL_STRING,  // String with NULL value. For test purposes only.

src/nvim/eval/funcs.c

@@ -5088,6 +5088,16 @@ static dict_T *create_environment(const dictitem_T *job_env, const bool clear_en
     tv_dict_add_str(env, S_LEN("TERM"), pty_term_name);
   }
 
+  // Set $NVIM (in the child process) to v:servername. #3118
+  char *nvim_addr = (char *)get_vim_var_str(VV_SEND_SERVER);
+  if (nvim_addr[0] != '\0') {
+    dictitem_T *dv = tv_dict_find(env, S_LEN("NVIM"));
+    if (dv) {
+      tv_dict_item_remove(env, dv);
+    }
+    tv_dict_add_str(env, S_LEN("NVIM"), nvim_addr);
+  }
+
   if (job_env) {
 #ifdef WIN32
     TV_DICT_ITER(job_env->di_tv.vval.v_dict, var, {

src/nvim/eval/typval.c

@@ -1588,8 +1588,6 @@ varnumber_T tv_dict_get_number(const dict_T *const d, const char *const key)
 }
 
 /// Converts a dict to an environment
-///
-///
 char **tv_dict_to_env(dict_T *denv)
 {
   size_t env_size = (size_t)tv_dict_len(denv);

src/nvim/msgpack_rpc/server.c

@@ -22,7 +22,8 @@
 #include "nvim/vim.h"
 
 #define MAX_CONNECTIONS 32
-#define LISTEN_ADDRESS_ENV_VAR "NVIM_LISTEN_ADDRESS"
+#define ENV_LISTEN "NVIM_LISTEN_ADDRESS"  // deprecated
+#define ENV_NVIM "NVIM"
 
 static garray_T watchers = GA_EMPTY_INIT_VALUE;
 
@@ -35,20 +36,24 @@ bool server_init(const char *listen_addr)
 {
   ga_init(&watchers, sizeof(SocketWatcher *), 1);
 
-  // $NVIM_LISTEN_ADDRESS
-  const char *env_addr = os_getenv(LISTEN_ADDRESS_ENV_VAR);
-  int rv = listen_addr == NULL ? 1 : server_start(listen_addr);
+  // $NVIM_LISTEN_ADDRESS (deprecated)
+  if (!listen_addr && os_env_exists(ENV_LISTEN)) {
+    listen_addr = os_getenv(ENV_LISTEN);
+  }
 
+  int rv = listen_addr ? server_start(listen_addr) : 1;
   if (0 != rv) {
-    rv = env_addr == NULL ? 1 : server_start(env_addr);
-    if (0 != rv) {
-      listen_addr = server_address_new();
-      if (listen_addr == NULL) {
-        return false;
-      }
-      rv = server_start(listen_addr);
-      xfree((char *)listen_addr);
+    listen_addr = server_address_new();
+    if (!listen_addr) {
+      return false;
     }
+    rv = server_start(listen_addr);
+    xfree((char *)listen_addr);
+  }
+
+  if (os_env_exists(ENV_LISTEN)) {
+    // Unset $NVIM_LISTEN_ADDRESS, it's a liability hereafter.
+    os_unsetenv(ENV_LISTEN);
   }
 
   return rv == 0;
@@ -60,8 +65,8 @@ static void close_socket_watcher(SocketWatcher **watcher)
   socket_watcher_close(*watcher, free_server);
 }
 
-/// Set v:servername to the first server in the server list, or unset it if no
-/// servers are known.
+/// Sets the "primary address" (v:servername and $NVIM) to the first server in
+/// the server list, or unsets if no servers are known.
 static void set_vservername(garray_T *srvs)
 {
   char *default_server = (srvs->ga_len > 0)
@@ -156,12 +161,6 @@ int server_start(const char *endpoint)
     return result;
   }
 
-  // Update $NVIM_LISTEN_ADDRESS, if not set.
-  const char *listen_address = os_getenv(LISTEN_ADDRESS_ENV_VAR);
-  if (listen_address == NULL) {
-    os_setenv(LISTEN_ADDRESS_ENV_VAR, watcher->addr, 1);
-  }
-
   // Add the watcher to the list.
   ga_grow(&watchers, 1);
   ((SocketWatcher **)watchers.ga_data)[watchers.ga_len++] = watcher;
@@ -200,12 +199,6 @@ bool server_stop(char *endpoint)
     return false;
   }
 
-  // Unset $NVIM_LISTEN_ADDRESS if it is the stopped address.
-  const char *listen_address = os_getenv(LISTEN_ADDRESS_ENV_VAR);
-  if (listen_address && STRCMP(addr, listen_address) == 0) {
-    os_unsetenv(LISTEN_ADDRESS_ENV_VAR);
-  }
-
   socket_watcher_close(watcher, free_server);
 
   // Remove this server from the list by swapping it with the last item.
@@ -215,8 +208,8 @@ bool server_stop(char *endpoint)
   }
   watchers.ga_len--;
 
-  // If v:servername is the stopped address, re-initialize it.
-  if (STRCMP(addr, get_vim_var_str(VV_SEND_SERVER)) == 0) {
+  // Bump v:servername to the next available server, if any.
+  if (strequal(addr, (char *)get_vim_var_str(VV_SEND_SERVER))) {
     set_vservername(&watchers);
   }
 

src/nvim/terminal.c

@@ -179,6 +179,13 @@ static void term_output_callback(const char *s, size_t len, void *user_data)
 
 // public API {{{
 
+/// Initializes terminal properties, and triggers TermOpen.
+///
+/// The PTY process (TerminalOptions.data) was already started by termopen(),
+/// via ex_terminal() or the term:// BufReadCmd.
+///
+/// @param buf Buffer used for presentation of the terminal.
+/// @param opts PTY process channel, various terminal properties and callbacks.
 Terminal *terminal_open(buf_T *buf, TerminalOptions opts)
 {
   // Create a new terminal instance and configure it
@@ -374,6 +381,7 @@ void terminal_check_size(Terminal *term)
   invalidate_terminal(term, -1, -1);
 }
 
+/// Implements TERM_FOCUS mode. :help Terminal-mode
 void terminal_enter(void)
 {
   buf_T *buf = curbuf;
@@ -502,6 +510,7 @@ static int terminal_check(VimState *state)
   return 1;
 }
 
+/// Processes one char of terminal-mode input.
 static int terminal_execute(VimState *state, int key)
 {
   TerminalState *s = (TerminalState *)state;
@@ -1448,7 +1457,8 @@ static void refresh_terminal(Terminal *term)
   long ml_added = buf->b_ml.ml_line_count - ml_before;
   adjust_topline(term, buf, ml_added);
 }
-// Calls refresh_terminal() on all invalidated_terminals.
+
+/// Calls refresh_terminal() on all invalidated_terminals.
 static void refresh_timer_cb(TimeWatcher *watcher, void *data)
 {
   refresh_pending = false;

src/nvim/terminal.h

@@ -13,7 +13,7 @@ typedef void (*terminal_close_cb)(void *data);
 #include "nvim/buffer_defs.h"
 
 typedef struct {
-  void *data;
+  void *data;  // PTY process channel
   uint16_t width, height;
   terminal_write_cb write_cb;
   terminal_resize_cb resize_cb;