From nobody Wed May 14 15:08:21 2025 Delivered-To: importer@patchew.org Received-SPF: pass (zoho.com: domain of redhat.com designates 209.132.183.28 as permitted sender) client-ip=209.132.183.28; envelope-from=libvir-list-bounces@redhat.com; helo=mx1.redhat.com; Authentication-Results: mx.zohomail.com; spf=pass (zoho.com: domain of redhat.com designates 209.132.183.28 as permitted sender) smtp.mailfrom=libvir-list-bounces@redhat.com; dmarc=pass(p=none dis=none) header.from=redhat.com Return-Path: Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by mx.zohomail.com with SMTPS id 152035914317257.254686190640655; Tue, 6 Mar 2018 09:59:03 -0800 (PST) Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.phx2.redhat.com [10.5.11.15]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id F1E2B757DC; Tue, 6 Mar 2018 17:59:01 +0000 (UTC) Received: from colo-mx.corp.redhat.com (colo-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.20]) by smtp.corp.redhat.com (Postfix) with ESMTPS id C81275D75A; Tue, 6 Mar 2018 17:59:01 +0000 (UTC) Received: from lists01.pubmisc.prod.ext.phx2.redhat.com (lists01.pubmisc.prod.ext.phx2.redhat.com [10.5.19.33]) by colo-mx.corp.redhat.com (Postfix) with ESMTP id 9047D18033EF; Tue, 6 Mar 2018 17:59:01 +0000 (UTC) Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.rdu2.redhat.com [10.11.54.5]) by lists01.pubmisc.prod.ext.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id w26HwxC2002072 for ; Tue, 6 Mar 2018 12:58:59 -0500 Received: by smtp.corp.redhat.com (Postfix) id 5F1451C71D; Tue, 6 Mar 2018 17:58:59 +0000 (UTC) Received: from t460.redhat.com (unknown [10.33.36.54]) by smtp.corp.redhat.com (Postfix) with ESMTP id CA98B1C720; Tue, 6 Mar 2018 17:58:58 +0000 (UTC) From: =?UTF-8?q?Daniel=20P=2E=20Berrang=C3=A9?= To: libvir-list@redhat.com Date: Tue, 6 Mar 2018 17:58:50 +0000 Message-Id: <20180306175852.18773-4-berrange@redhat.com> In-Reply-To: <20180306175852.18773-1-berrange@redhat.com> References: <20180306175852.18773-1-berrange@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.11.54.5 X-loop: libvir-list@redhat.com Subject: [libvirt] [PATCH 3/5] rpc: invoke the message dispatch callback with client unlocked X-BeenThere: libvir-list@redhat.com X-Mailman-Version: 2.1.12 Precedence: junk List-Id: Development discussions about the libvirt library & tools List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable Sender: libvir-list-bounces@redhat.com Errors-To: libvir-list-bounces@redhat.com X-Scanned-By: MIMEDefang 2.79 on 10.5.11.15 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.26]); Tue, 06 Mar 2018 17:59:02 +0000 (UTC) X-ZohoMail: RSF_0 Z_629925259 SPT_0 Currently if the virNetServer instance is created with max_workers=3D=3D0 to request a non-threaded dispatch process, we deadlock during dispatch #0 0x00007fb845f6f42d in __lll_lock_wait () from /lib64/libpthread.so.0 #1 0x00007fb845f681d3 in pthread_mutex_lock () from /lib64/libpthread.so= .0 #2 0x000055a6628bb305 in virMutexLock (m=3D) at util/virt= hread.c:89 #3 0x000055a6628a984b in virObjectLock (anyobj=3D) at uti= l/virobject.c:435 #4 0x000055a66286fcde in virNetServerClientIsAuthenticated (client=3Dcli= ent@entry=3D0x55a663a7b960) at rpc/virnetserverclient.c:1565 #5 0x000055a66286cc17 in virNetServerProgramDispatchCall (msg=3D0x55a663= a7bc50, client=3D0x55a663a7b960, server=3D0x55a663a77550, prog=3D0x55a663a78020) at rpc/virnetserverpr= ogram.c:407 #6 virNetServerProgramDispatch (prog=3Dprog@entry=3D0x55a663a78020, serv= er=3Dserver@entry=3D0x55a663a77550, client=3Dclient@entry=3D0x55a663a7b960, msg=3Dmsg@entry=3D0x55a663a7b= c50) at rpc/virnetserverprogram.c:307 #7 0x000055a662871d56 in virNetServerProcessMsg (msg=3D0x55a663a7bc50, p= rog=3D0x55a663a78020, client=3D0x55a663a7b960, srv=3D0x55a663a77550) at rpc/virnetserver.c:148 #8 virNetServerDispatchNewMessage (client=3D0x55a663a7b960, msg=3D0x55a6= 63a7bc50, opaque=3D0x55a663a77550) at rpc/virnetserver.c:227 #9 0x000055a66286e4c0 in virNetServerClientDispatchRead (client=3Dclient= @entry=3D0x55a663a7b960) at rpc/virnetserverclient.c:1322 #10 0x000055a66286e813 in virNetServerClientDispatchEvent (sock=3D, events=3D1, opaque=3D0x55a663a7b960) at rpc/virnetserverclient.c:1507 #11 0x000055a662899be0 in virEventPollDispatchHandles (fds=3D0x55a663a7bd= c0, nfds=3D) at util/vireventpoll.c:508 #12 virEventPollRunOnce () at util/vireventpoll.c:657 #13 0x000055a6628982f1 in virEventRunDefaultImpl () at util/virevent.c:327 #14 0x000055a6628716d5 in virNetDaemonRun (dmn=3D0x55a663a771b0) at rpc/v= irnetdaemon.c:858 #15 0x000055a662864c1d in main (argc=3D, argv=3D0x7ffd105b= 4838) at logging/log_daemon.c:1235 Signed-off-by: Daniel P. Berrang=C3=A9 Reviewed-by: Jim Fehlig Reviewed-by: John Ferlan --- src/rpc/virnetserverclient.c | 79 ++++++++++++++++++++++++++++++----------= ---- 1 file changed, 55 insertions(+), 24 deletions(-) diff --git a/src/rpc/virnetserverclient.c b/src/rpc/virnetserverclient.c index ea0d5abdee..fb4775235d 100644 --- a/src/rpc/virnetserverclient.c +++ b/src/rpc/virnetserverclient.c @@ -143,7 +143,7 @@ VIR_ONCE_GLOBAL_INIT(virNetServerClient) =20 static void virNetServerClientDispatchEvent(virNetSocketPtr sock, int even= ts, void *opaque); static void virNetServerClientUpdateEvent(virNetServerClientPtr client); -static void virNetServerClientDispatchRead(virNetServerClientPtr client); +static virNetMessagePtr virNetServerClientDispatchRead(virNetServerClientP= tr client); static int virNetServerClientSendMessageLocked(virNetServerClientPtr clien= t, virNetMessagePtr msg); =20 @@ -340,18 +340,41 @@ virNetServerClientCheckAccess(virNetServerClientPtr c= lient) } #endif =20 +static void virNetServerClientDispatchMessage(virNetServerClientPtr client, + virNetMessagePtr msg) +{ + virObjectLock(client); + if (!client->dispatchFunc) { + virNetMessageFree(msg); + client->wantClose =3D true; + virObjectUnlock(client); + } else { + virObjectUnlock(client); + /* Accessing 'client' is safe, because virNetServerClientSetDispat= cher + * only permits setting 'dispatchFunc' once, so if non-NULL, it wi= ll + * never change again + */ + client->dispatchFunc(client, msg, client->dispatchOpaque); + } +} + =20 static void virNetServerClientSockTimerFunc(int timer, void *opaque) { virNetServerClientPtr client =3D opaque; + virNetMessagePtr msg =3D NULL; virObjectLock(client); virEventUpdateTimeout(timer, -1); /* Although client->rx !=3D NULL when this timer is enabled, it might = have * changed since the client was unlocked in the meantime. */ if (client->rx) - virNetServerClientDispatchRead(client); + msg =3D virNetServerClientDispatchRead(client); virObjectUnlock(client); + + if (msg) { + virNetServerClientDispatchMessage(client, msg); + } } =20 =20 @@ -950,8 +973,13 @@ void virNetServerClientSetDispatcher(virNetServerClien= tPtr client, void *opaque) { virObjectLock(client); - client->dispatchFunc =3D func; - client->dispatchOpaque =3D opaque; + /* Only set dispatcher if not already set, to avoid race + * with dispatch code that runs without locks held + */ + if (!client->dispatchFunc) { + client->dispatchFunc =3D func; + client->dispatchOpaque =3D opaque; + } virObjectUnlock(client); } =20 @@ -1196,26 +1224,32 @@ static ssize_t virNetServerClientRead(virNetServerC= lientPtr client) =20 =20 /* - * Read data until we get a complete message to process + * Read data until we get a complete message to process. + * If a complete message is available, it will be returned + * from this method, for dispatch by the caller. + * + * Returns a complete message for dispatch, or NULL if none is + * yet available, or an error occurred. On error, the wantClose + * flag will be set. */ -static void virNetServerClientDispatchRead(virNetServerClientPtr client) +static virNetMessagePtr virNetServerClientDispatchRead(virNetServerClientP= tr client) { readmore: if (client->rx->nfds =3D=3D 0) { if (virNetServerClientRead(client) < 0) { client->wantClose =3D true; - return; /* Error */ + return NULL; /* Error */ } } =20 if (client->rx->bufferOffset < client->rx->bufferLength) - return; /* Still not read enough */ + return NULL; /* Still not read enough */ =20 /* Either done with length word header */ if (client->rx->bufferLength =3D=3D VIR_NET_MESSAGE_LEN_MAX) { if (virNetMessageDecodeLength(client->rx) < 0) { client->wantClose =3D true; - return; + return NULL; } =20 virNetServerClientUpdateEvent(client); @@ -1236,7 +1270,7 @@ static void virNetServerClientDispatchRead(virNetServ= erClientPtr client) virNetMessageQueueServe(&client->rx); virNetMessageFree(msg); client->wantClose =3D true; - return; + return NULL; } =20 /* Now figure out if we need to read more data to get some @@ -1246,7 +1280,7 @@ static void virNetServerClientDispatchRead(virNetServ= erClientPtr client) virNetMessageQueueServe(&client->rx); virNetMessageFree(msg); client->wantClose =3D true; - return; /* Error */ + return NULL; /* Error */ } =20 /* Try getting the file descriptors (may fail if blocking) */ @@ -1256,7 +1290,7 @@ static void virNetServerClientDispatchRead(virNetServ= erClientPtr client) virNetMessageQueueServe(&client->rx); virNetMessageFree(msg); client->wantClose =3D true; - return; + return NULL; } if (rv =3D=3D 0) /* Blocking */ break; @@ -1270,7 +1304,7 @@ static void virNetServerClientDispatchRead(virNetServ= erClientPtr client) * again next time we run this method */ client->rx->bufferOffset =3D client->rx->bufferLength; - return; + return NULL; } } =20 @@ -1313,16 +1347,6 @@ static void virNetServerClientDispatchRead(virNetSer= verClientPtr client) } } =20 - /* Send off to for normal dispatch to workers */ - if (msg) { - if (!client->dispatchFunc) { - virNetMessageFree(msg); - client->wantClose =3D true; - } else { - client->dispatchFunc(client, msg, client->dispatchOpaque); - } - } - /* Possibly need to create another receive buffer */ if (client->nrequests < client->nrequests_max) { if (!(client->rx =3D virNetMessageNew(true))) { @@ -1338,6 +1362,8 @@ static void virNetServerClientDispatchRead(virNetServ= erClientPtr client) } } virNetServerClientUpdateEvent(client); + + return msg; } } =20 @@ -1482,6 +1508,7 @@ static void virNetServerClientDispatchEvent(virNetSocketPtr sock, int events, void *op= aque) { virNetServerClientPtr client =3D opaque; + virNetMessagePtr msg =3D NULL; =20 virObjectLock(client); =20 @@ -1504,7 +1531,7 @@ virNetServerClientDispatchEvent(virNetSocketPtr sock,= int events, void *opaque) virNetServerClientDispatchWrite(client); if (events & VIR_EVENT_HANDLE_READABLE && client->rx) - virNetServerClientDispatchRead(client); + msg =3D virNetServerClientDispatchRead(client); #if WITH_GNUTLS } #endif @@ -1517,6 +1544,10 @@ virNetServerClientDispatchEvent(virNetSocketPtr sock= , int events, void *opaque) client->wantClose =3D true; =20 virObjectUnlock(client); + + if (msg) { + virNetServerClientDispatchMessage(client, msg); + } } =20 =20 --=20 2.14.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list