Hello, 

Your patch works on our stand without any freezing.

Tested by: Fima Shevrin <efim.shevrin@virtuozzo.com>

From: Daniel P. Berrangé <berrange@redhat.com>
Sent: Monday, December 18, 2023 20:23
To: devel@lists.libvirt.org <devel@lists.libvirt.org>
Cc: Efim Shevrin <efim.shevrin@virtuozzo.com>; Denis V. Lunev <den@openvz.org>; Daniel P. Berrangé <berrange@redhat.com>
Subject: [PATCH] rpc: fix race in waking up client event loop
 
[You don't often get email from berrange@redhat.com. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]

The first thread to issue a client RPC request will own the event
loop execution, sitting in the virNetClientIOEventLoop function.

It releases the client lock while running:

   virNetClientUnlock()
   g_main_loop_run()
   virNetClientLock()

If a second thread arrives with an RPC request, it will queue it
for the first thread to process. To inform the first thread that
there's a new request it calls g_main_loop_quit() to break it out
of the main loop.

This works if the first thread is in g_main_loop_run() at that
time. There is a small window of opportunity, however, where
the first thread has released the client lock, but not yet got
into g_main_loop_run(). If that happens, the wakeup from the
second thread is lost.

This patch deals with that by changing the way the wakeup is
performed. Instead of directly calling g_main_loop_quit(), the
second thread creates an idle source to run the quit function
from within the first thread. This guarantees that the first
thread will see the wakeup.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---

 src/rpc/virnetclient.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/src/rpc/virnetclient.c b/src/rpc/virnetclient.c
index 4ab8af68c5..68098b1c8d 100644
--- a/src/rpc/virnetclient.c
+++ b/src/rpc/virnetclient.c
@@ -1848,6 +1848,15 @@ static void virNetClientIOUpdateCallback(virNetClient *client,
 }


+static gboolean virNetClientIOWakeup(gpointer opaque)
+{
+    GMainLoop *loop = opaque;
+
+    g_main_loop_quit(loop);
+
+    return G_SOURCE_REMOVE;
+}
+
 /*
  * This function sends a message to remote server and awaits a reply
  *
@@ -1925,7 +1934,9 @@ static int virNetClientIO(virNetClient *client,
     /* Check to see if another thread is dispatching */
     if (client->haveTheBuck) {
         /* Force other thread to wakeup from poll */
-        g_main_loop_quit(client->eventLoop);
+        GSource *wakeup = g_idle_source_new();
+        g_source_set_callback(wakeup, virNetClientIOWakeup, client->eventLoop, NULL);
+        g_source_attach(wakeup, client->eventCtx);

         /* If we are non-blocking, detach the thread and keep the call in the
          * queue. */
--
2.43.0