From b8d83cd7f4211b48ab94e9d1208c40607e7c3a2f Mon Sep 17 00:00:00 2001
From: Vitaliy Filippov <vitalif@yourcmc.ru>
Date: Sat, 22 Feb 2025 15:09:00 +0300
Subject: [PATCH] No, it's not a good idea to destroy client in the child nbd
 process

Should probably have been an obvious side effect :-)

Child process gets open file descriptors to parent's epoll/timerfd,
and it's totally OK to just close() all of them, but it's absolutely NOT
OK to run destructors - they modify the kernel state of epoll/timerfd
before destroying. So, basically, when we destroy the client in the child
process, we break it in the parent too. This also means that cluster_client_t
doesn't support fork(). :-)
---
 src/client/nbd_proxy.cpp | 23 +++++++----------------
 1 file changed, 7 insertions(+), 16 deletions(-)

diff --git a/src/client/nbd_proxy.cpp b/src/client/nbd_proxy.cpp
index 458a82bd..ec98b426 100644
--- a/src/client/nbd_proxy.cpp
+++ b/src/client/nbd_proxy.cpp
@@ -709,21 +709,13 @@ help:
             ringloop->loop();
             ringloop->wait();
         }
-        destroy_client();
-    }
-
-    void destroy_client()
-    {
-        if (cli)
-        {
-            cli->flush();
-            delete cli;
-            delete epmgr;
-            delete ringloop;
-            cli = NULL;
-            epmgr = NULL;
-            ringloop = NULL;
-        }
+        cli->flush();
+        delete cli;
+        delete epmgr;
+        delete ringloop;
+        cli = NULL;
+        epmgr = NULL;
+        ringloop = NULL;
     }
 
     void load_module()
@@ -911,7 +903,6 @@ protected:
             // works correctly, and we don't accidentally break previous NBD devices while setting
             // up a new device. Forking to check every device is of course rather slow, so we also
             // do an additional check by calling list_mapped() before searching for a free NBD device.
-            destroy_client();
             if (bg)
             {
                 daemonize_fork();