Re: [libvirt] [PATCH] add lock priv->lock protection in remoteClientCloseFunc()

sometimes, virsh console session may closed by virsh console existed session(when virsh console client exit) and new virsh console --force session simutaneously. So in one thread(Job worker), it calls daemonStreamEvent() and referencing stream, and in another thread(main thread), virNetServerClientClose() was called, and the callback remoteClientCloseFunc() was called, without protect by priv->lock in remoteClientCloseFunc(), it may lead to daemonStreamEvent reference stream released by remoteClientCloseFunc(), and also need change virNetServerClientClose() to be virNetServerClientImmediateClose() in daemonStreamEvent(), or it lead to libvirt daemon deadlock by double lock priv->lock in daemonStreamEvent() --- src/remote/remote_daemon_dispatch.c | 2 ++ src/remote/remote_daemon_stream.c | 14 +++++++------- 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/src/remote/remote_daemon_dispatch.c b/src/remote/remote_daemon_dispatch.c index f369ffb..b0982b7 100644 --- a/src/remote/remote_daemon_dispatch.c +++ b/src/remote/remote_daemon_dispatch.c @@ -1724,7 +1724,9 @@ static void remoteClientCloseFunc(virNetServerClientPtr client) { struct daemonClientPrivate *priv = virNetServerClientGetPrivateData(client); + virMutexLock(&priv->lock); daemonRemoveAllClientStreams(priv->streams); + virMutexUnlock(&priv->lock); remoteClientFreePrivateCallbacks(priv); } diff --git a/src/remote/remote_daemon_stream.c b/src/remote/remote_daemon_stream.c index 73e4d7b..6a84fdf 100644 --- a/src/remote/remote_daemon_stream.c +++ b/src/remote/remote_daemon_stream.c @@ -141,7 +141,7 @@ daemonStreamEvent(virStreamPtr st, int events, void *opaque) (events & VIR_STREAM_EVENT_WRITABLE)) { if (daemonStreamHandleWrite(client, stream) < 0) { daemonRemoveClientStream(client, stream); - virNetServerClientClose(client); + virNetServerClientImmediateClose(client); goto cleanup; } } @@ -151,7 +151,7 @@ daemonStreamEvent(virStreamPtr st, int events, void *opaque) events = events & ~(VIR_STREAM_EVENT_READABLE); if (daemonStreamHandleRead(client, stream) < 0) { daemonRemoveClientStream(client, stream); - virNetServerClientClose(client); + virNetServerClientImmediateClose(client); goto cleanup; } /* If we detected EOF during read processing, @@ -176,7 +176,7 @@ daemonStreamEvent(virStreamPtr st, int events, void *opaque) if (daemonStreamHandleFinish(client, stream, msg) < 0) { virNetMessageFree(msg); daemonRemoveClientStream(client, stream); - virNetServerClientClose(client); + virNetServerClientImmediateClose(client); goto cleanup; } break; @@ -186,7 +186,7 @@ daemonStreamEvent(virStreamPtr st, int events, void *opaque) if (daemonStreamHandleAbort(client, stream, msg) < 0) { virNetMessageFree(msg); daemonRemoveClientStream(client, stream); - virNetServerClientClose(client); + virNetServerClientImmediateClose(client); goto cleanup; } break; @@ -205,7 +205,7 @@ daemonStreamEvent(virStreamPtr st, int events, void *opaque) stream->recvEOF = true; if (!(msg = virNetMessageNew(false))) { daemonRemoveClientStream(client, stream); - virNetServerClientClose(client); + virNetServerClientImmediateClose(client); goto cleanup; } msg->cb = daemonStreamMessageFinished; @@ -219,7 +219,7 @@ daemonStreamEvent(virStreamPtr st, int events, void *opaque) "", 0) < 0) { virNetMessageFree(msg); daemonRemoveClientStream(client, stream); - virNetServerClientClose(client); + virNetServerClientImmediateClose(client); goto cleanup; } } @@ -262,7 +262,7 @@ daemonStreamEvent(virStreamPtr st, int events, void *opaque) } daemonRemoveClientStream(client, stream); if (ret < 0) - virNetServerClientClose(client); + virNetServerClientImmediateClose(client); goto cleanup; } -- 1.8.3.1