From 883b6b7700999f3c7f7bddd87f34ca028fe9977e Mon Sep 17 00:00:00 2001
From: Robbert van der Helm <mail@robbertvanderhelm.nl>
Date: Sun, 16 May 2021 15:24:31 +0200
Subject: [PATCH] Manually close descriptors instead of using vfork

With `vfork()` the child process inherits the parents process image and
prevents copying them, but if it outlives its parent then the file
descriptors will still remain open. Manually closing all file
descriptors is the only solution here.

This was only an issue with Ardour since they don't open all of their
files with `FD_CLOEXEC`. Last update's watchdog timer somewhat mitigated
the issue, but Ardour should now no longer freeze when reopening because
of this. The watchdog timer is still necessary, since hanging Wine
processes will still prevent the Wine server from shutting down.
---
 CHANGELOG.md                |  6 ++++++
 src/plugin/host-process.cpp | 19 +++++++++++++++----
 src/plugin/host-process.h   |  3 ---
 3 files changed, 21 insertions(+), 7 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index a21f80cb..593f3376 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -31,6 +31,12 @@ Versioning](https://semver.org/spec/v2.0.0.html).
 
 ### Fixed
 
+- Fixed yabridge's Wine processes inheriting file descriptors in some
+  situations. This could cause **Ardour** and **Mixbus** to hang when trying to
+  reopen it after a crash. The watchdog timer added in yabridge 3.2.0 also
+  addressed this issue partially, but it should now be completely fixed. This
+  may also prevent rare issues where the **JACK** server would hang after the
+  host crashes.
 - Fixed _DMG_ VST3 plugins freezing in **REAPER** when the plugin resizes itself
   while the host passes channel context information to the plugin.
 - Also fixed _DMG_ VST3 plugins freezing in **REAPER** when restoring multiple
diff --git a/src/plugin/host-process.cpp b/src/plugin/host-process.cpp
index ca61be61..b459c0c6 100644
--- a/src/plugin/host-process.cpp
+++ b/src/plugin/host-process.cpp
@@ -18,6 +18,7 @@
 
 #include <boost/asio/read_until.hpp>
 #include <boost/process/env.hpp>
+#include <boost/process/extend.hpp>
 #include <boost/process/io.hpp>
 #include <boost/process/start_dir.hpp>
 
@@ -44,10 +45,20 @@ bp::child launch_host(fs::path host_path, Args&&... args) {
 #else
         host_path,
 #endif
-        // We'll use vfork() instead of fork to avoid potential issues with
-        // inheriting file descriptors
-        // https://github.com/robbert-vdh/yabridge/issues/45
-        bp::posix::use_vfork, std::forward<Args>(args)...);
+        // NOTE: If the Wine process outlives the host, then it may cause issues
+        //       if our process is still keeping the host's file descriptors
+        //       alive that. This can prevent Ardour from restarting after an
+        //       unexpected shutdown. Because of this we won't use `vfork()`,
+        //       but instead we'll just manually close all non-STDIO file
+        //       descriptors.
+        bp::extend::on_exec_setup =
+            [](auto& /*executor*/) {
+                const int max_fds = static_cast<int>(sysconf(_SC_OPEN_MAX));
+                for (int fd = STDERR_FILENO + 1; fd < max_fds; fd++) {
+                    close(fd);
+                }
+            },
+        std::forward<Args>(args)...);
 }
 
 HostProcess::HostProcess(boost::asio::io_context& io_context,
diff --git a/src/plugin/host-process.h b/src/plugin/host-process.h
index 074ab859..bd776852 100644
--- a/src/plugin/host-process.h
+++ b/src/plugin/host-process.h
@@ -18,9 +18,6 @@
 
 #include <thread>
 
-// Boost.Process's auto detection for vfork() support doesn't seem to work
-#define BOOST_POSIX_HAS_VFORK 1
-
 #include <boost/asio/local/stream_protocol.hpp>
 #include <boost/asio/streambuf.hpp>
 #include <boost/filesystem.hpp>