wstream/shell: Fix memory errors caused by os_system

The os_system function uses a write callback to close the input stream when the
write completes, but this causes a memory error because the callback is invoked
right before the stream is freed by the caller.

This fixes the problem by removing the callback set by os_system. Instead, it
calls job_close_in immediately after writing(the stream will only close after
the write completes). The 'pending' parameter was also removed from the
'write_cb' as it should be hidden by the wstream module.

While the `wstream_set_write_cb` and `job_write_cb` are no longer used, they
will remain in the codebase for future use.

3 files changed, 4 insertions, 22 deletions

diff --git a/src/nvim/os/shell.c b/src/nvim/os/shell.c
index 398d94b606..912dc95aca 100644
--- a/src/nvim/os/shell.c
+++ b/src/nvim/os/shell.c
@@ -293,19 +293,15 @@ int os_system(const char *cmd,
   if (input) {
     WBuffer *input_buffer = wstream_new_buffer((char *) input, len, 1, NULL);
 
-    // we want to be notified when the write completes
-    job_write_cb(job, system_write_cb);
-
     if (!job_write(job, input_buffer)) {
       // couldn't write, stop the job and tell the user about it
       job_stop(job);
       return -1;
     }
-  } else {
-    // close the input stream, let the process know that no input is coming
-    job_close_in(job);
   }
 
+  // close the input stream, let the process know that no more input is coming
+  job_close_in(job);
   int status = job_wait(job, -1);
 
   // prepare the out parameters if requested
@@ -353,17 +349,6 @@ static void system_data_cb(RStream *rstream, void *data, bool eof)
   buf->len += nread;
 }
 
-static void system_write_cb(WStream *wstream,
-                            void *data,
-                            size_t pending,
-                            int status)
-{
-  if (pending == 0) {
-    Job *job = data;
-    job_close_in(job);
-  }
-}
-
 /// Parses a command string into a sequence of words, taking quotes into
 /// consideration.
 ///
diff --git a/src/nvim/os/wstream.c b/src/nvim/os/wstream.c
index 00a53d1628..eb7de02a2f 100644
--- a/src/nvim/os/wstream.c
+++ b/src/nvim/os/wstream.c
@@ -208,15 +208,14 @@ static void write_cb(uv_write_t *req, int status)
 
   release_wbuffer(data->buffer);
 
-  data->wstream->pending_reqs--;
-
   if (data->wstream->cb) {
     data->wstream->cb(data->wstream,
                       data->wstream->data,
-                      data->wstream->pending_reqs,
                       status);
   }
 
+  data->wstream->pending_reqs--;
+
   if (data->wstream->freed && data->wstream->pending_reqs == 0) {
     // Last pending write, free the wstream;
     free(data->wstream);
diff --git a/src/nvim/os/wstream_defs.h b/src/nvim/os/wstream_defs.h
index e42481f283..cfa0bf0b60 100644
--- a/src/nvim/os/wstream_defs.h
+++ b/src/nvim/os/wstream_defs.h
@@ -10,11 +10,9 @@ typedef void (*wbuffer_data_finalizer)(void *data);
 ///
 /// @param wstream The `WStream` instance
 /// @param data User-defined data
-/// @param pending The number of write requests that are still pending
 /// @param status 0 on success, anything else indicates failure
 typedef void (*wstream_cb)(WStream *wstream,
                            void *data,
-                           size_t pending,
                            int status);
 
 #endif  // NVIM_OS_WSTREAM_DEFS_H