refactor(fs): now it is time to get rid of fs_loop and fs_loop_mutex

Here's the headline: when run in sync mode (last argument cb=NULL),
these functions don't actually use the uv_loop_t.

An earlier version of this patch instead replaced fs_loop with
using main_loop.uv on the main thread and luv_loop() on luv worker
threads. However this made the code more complicated for no reason.

Also arbitrarily, half of these functions would attempt to handle
UV_ENOMEM by try_to_free_memory(). This would mostly happen
on windows because it needs to allocate a converted WCHAR buffer.
This should be a quite rare situation. Your system is pretty
much hosed already if you cannot allocate like 50 WCHAR:s.
Therefore, take the liberty of simply removing this fallback.

In addition, we tried to "recover" from ENOMEM in read()/readv()
this way which doesn't make any sense. The read buffer(s) are already
allocated at this point.

This would also be an issue when using these functions on a worker
thread, as try_to_free_memory() is not thread-safe. Currently
os_file_is_readable() and os_is_dir() is used by worker threads
(as part of nvim__get_runtime(), to implement require from 'rtp' in
threads).

In the end, these changes makes _all_ os/fs.c functions thread-safe,
and we thus don't need to document and maintain a thread-safe subset.

src/nvim/main.c

@@ -266,10 +266,6 @@ int main(int argc, char **argv)
   // `argc` and `argv` are also copied, so that they can be changed.
   init_params(&params, argc, argv);
 
-  // Since os_open is called during the init_startuptime, we need to call
-  // fs_init before it.
-  fs_init();
-
   init_startuptime(&params);
 
   // Need to find "--clean" before actually parsing arguments.
@@ -1479,7 +1475,7 @@ static void init_startuptime(mparm_T *paramp)
 {
   for (int i = 1; i < paramp->argc - 1; i++) {
     if (STRICMP(paramp->argv[i], "--startuptime") == 0) {
-      time_fd = os_fopen(paramp->argv[i + 1], "a");
+      time_fd = fopen(paramp->argv[i + 1], "a");
       time_start("--- NVIM STARTING ---");
       break;
     }

src/nvim/os/fs.c

@@ -20,6 +20,7 @@
 #include "nvim/globals.h"
 #include "nvim/log.h"
 #include "nvim/macros.h"
+#include "nvim/main.h"
 #include "nvim/memory.h"
 #include "nvim/message.h"
 #include "nvim/option_defs.h"
@@ -46,44 +47,13 @@ struct iovec;
 
 #define RUN_UV_FS_FUNC(ret, func, ...) \
   do { \
-    bool did_try_to_free = false; \
-uv_call_start: {} \
     uv_fs_t req; \
-    fs_loop_lock(); \
-    ret = func(&fs_loop, &req, __VA_ARGS__); \
+    ret = func(NULL, &req, __VA_ARGS__); \
     uv_fs_req_cleanup(&req); \
-    fs_loop_unlock(); \
-    if (ret == UV_ENOMEM && !did_try_to_free) { \
-      try_to_free_memory(); \
-      did_try_to_free = true; \
-      goto uv_call_start; \
-    } \
   } while (0)
 
 // Many fs functions from libuv return that value on success.
 static const int kLibuvSuccess = 0;
-static uv_loop_t fs_loop;
-static uv_mutex_t fs_loop_mutex;
-
-// Initialize the fs module
-void fs_init(void)
-{
-  uv_loop_init(&fs_loop);
-  uv_mutex_init_recursive(&fs_loop_mutex);
-}
-
-/// TODO(bfredl): some of these operations should
-/// be possible to do the private libuv loop of the
-/// thread, instead of contending the global fs loop
-void fs_loop_lock(void)
-{
-  uv_mutex_lock(&fs_loop_mutex);
-}
-
-void fs_loop_unlock(void)
-{
-  uv_mutex_unlock(&fs_loop_mutex);
-}
 
 /// Changes the current directory to `path`.
 ///
@@ -122,12 +92,9 @@ bool os_isrealdir(const char *name)
   FUNC_ATTR_NONNULL_ALL
 {
   uv_fs_t request;
-  fs_loop_lock();
-  if (uv_fs_lstat(&fs_loop, &request, name, NULL) != kLibuvSuccess) {
-    fs_loop_unlock();
+  if (uv_fs_lstat(NULL, &request, name, NULL) != kLibuvSuccess) {
     return false;
   }
-  fs_loop_unlock();
   if (S_ISLNK(request.statbuf.st_mode)) {
     return false;
   }
@@ -566,7 +533,6 @@ ptrdiff_t os_read(const int fd, bool *const ret_eof, char *const ret_buf, const
     return 0;
   }
   size_t read_bytes = 0;
-  bool did_try_to_free = false;
   while (read_bytes != size) {
     assert(size >= read_bytes);
     const ptrdiff_t cur_read_bytes = read(fd, ret_buf + read_bytes,
@@ -581,10 +547,6 @@ ptrdiff_t os_read(const int fd, bool *const ret_eof, char *const ret_buf, const
         break;
       } else if (error == UV_EINTR || error == UV_EAGAIN) {
         continue;
-      } else if (error == UV_ENOMEM && !did_try_to_free) {
-        try_to_free_memory();
-        did_try_to_free = true;
-        continue;
       } else {
         return (ptrdiff_t)error;
       }
@@ -618,7 +580,6 @@ ptrdiff_t os_readv(const int fd, bool *const ret_eof, struct iovec *iov, size_t
 {
   *ret_eof = false;
   size_t read_bytes = 0;
-  bool did_try_to_free = false;
   size_t toread = 0;
   for (size_t i = 0; i < iov_size; i++) {
     // Overflow, trying to read too much data
@@ -650,10 +611,6 @@ ptrdiff_t os_readv(const int fd, bool *const ret_eof, struct iovec *iov, size_t
         break;
       } else if (error == UV_EINTR || error == UV_EAGAIN) {
         continue;
-      } else if (error == UV_ENOMEM && !did_try_to_free) {
-        try_to_free_memory();
-        did_try_to_free = true;
-        continue;
       } else {
         return (ptrdiff_t)error;
       }
@@ -742,9 +699,7 @@ static int os_stat(const char *name, uv_stat_t *statbuf)
     return UV_EINVAL;
   }
   uv_fs_t request;
-  fs_loop_lock();
-  int result = uv_fs_stat(&fs_loop, &request, name, NULL);
-  fs_loop_unlock();
+  int result = uv_fs_stat(NULL, &request, name, NULL);
   if (result == kLibuvSuccess) {
     *statbuf = request.statbuf;
   }
@@ -1028,9 +983,7 @@ int os_mkdtemp(const char *templ, char *path)
   FUNC_ATTR_NONNULL_ALL
 {
   uv_fs_t request;
-  fs_loop_lock();
-  int result = uv_fs_mkdtemp(&fs_loop, &request, templ, NULL);
-  fs_loop_unlock();
+  int result = uv_fs_mkdtemp(NULL, &request, templ, NULL);
   if (result == kLibuvSuccess) {
     xstrlcpy(path, request.path, TEMP_FILE_PATH_MAXLEN);
   }
@@ -1057,9 +1010,7 @@ int os_rmdir(const char *path)
 bool os_scandir(Directory *dir, const char *path)
   FUNC_ATTR_NONNULL_ALL
 {
-  fs_loop_lock();
-  int r = uv_fs_scandir(&fs_loop, &dir->request, path, 0, NULL);
-  fs_loop_unlock();
+  int r = uv_fs_scandir(NULL, &dir->request, path, 0, NULL);
   if (r < 0) {
     os_closedir(dir);
   }
@@ -1120,9 +1071,7 @@ bool os_fileinfo_link(const char *path, FileInfo *file_info)
     return false;
   }
   uv_fs_t request;
-  fs_loop_lock();
-  bool ok = uv_fs_lstat(&fs_loop, &request, path, NULL) == kLibuvSuccess;
-  fs_loop_unlock();
+  bool ok = uv_fs_lstat(NULL, &request, path, NULL) == kLibuvSuccess;
   if (ok) {
     file_info->stat = request.statbuf;
   }
@@ -1140,8 +1089,7 @@ bool os_fileinfo_fd(int file_descriptor, FileInfo *file_info)
 {
   uv_fs_t request;
   CLEAR_POINTER(file_info);
-  fs_loop_lock();
-  bool ok = uv_fs_fstat(&fs_loop,
+  bool ok = uv_fs_fstat(NULL,
                         &request,
                         file_descriptor,
                         NULL) == kLibuvSuccess;
@@ -1149,7 +1097,6 @@ bool os_fileinfo_fd(int file_descriptor, FileInfo *file_info)
     file_info->stat = request.statbuf;
   }
   uv_fs_req_cleanup(&request);
-  fs_loop_unlock();
   return ok;
 }
 
@@ -1266,8 +1213,7 @@ char *os_realpath(const char *name, char *buf)
   FUNC_ATTR_NONNULL_ARG(1)
 {
   uv_fs_t request;
-  fs_loop_lock();
-  int result = uv_fs_realpath(&fs_loop, &request, name, NULL);
+  int result = uv_fs_realpath(NULL, &request, name, NULL);
   if (result == kLibuvSuccess) {
     if (buf == NULL) {
       buf = xmallocz(MAXPATHL);
@@ -1275,7 +1221,6 @@ char *os_realpath(const char *name, char *buf)
     xstrlcpy(buf, request.ptr, MAXPATHL + 1);
   }
   uv_fs_req_cleanup(&request);
-  fs_loop_unlock();
   return result == kLibuvSuccess ? buf : NULL;
 }
 

test/unit/helpers.lua

@@ -97,7 +97,6 @@ local init = only_separate(function()
   for _, c in ipairs(child_calls_init) do
     c.func(unpack(c.args))
   end
-  libnvim.fs_init()
   libnvim.event_init()
   libnvim.early_init(nil)
   if child_calls_mod then