|
Hello,
Your patch works on our stand without any freezing. Tested by: Fima Shevrin <efim.shevrin@xxxxxxxxxxxxx> From: Daniel P. Berrangé <berrange@xxxxxxxxxx>
Sent: Monday, December 18, 2023 20:23 To: devel@xxxxxxxxxxxxxxxxx <devel@xxxxxxxxxxxxxxxxx> Cc: Efim Shevrin <efim.shevrin@xxxxxxxxxxxxx>; Denis V. Lunev <den@xxxxxxxxxx>; Daniel P. Berrangé <berrange@xxxxxxxxxx> Subject: [PATCH] rpc: fix race in waking up client event loop [You don't often get email from berrange@xxxxxxxxxx. 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@xxxxxxxxxx> --- 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 |
_______________________________________________ Devel mailing list -- devel@xxxxxxxxxxxxxxxxx To unsubscribe send an email to devel-leave@xxxxxxxxxxxxxxxxx
