[PATCH 1/9] rpc: fix race on stream abort/finish and server side abort

[Nikolay Shirokovskiy <nshirokovskiy@xxxxxxxxxxxxx>]

Stream abort/finish can hang because we can receive abort message
from server and yet sent abort/finish message to server. The latter
will not be answered ever because after server sends abort message
it forgets the stream and messages for unknown stream are simply ignored.

We check for stream error at the very beginning of remoteStreamFinish/remoteStreamAbort
but stream error can be set after the check in another thread operating
on stream. Let's check for stream error under client lock similar
to what's done in [1].

[1] 833b901cb: stream: Check for stream EOF

Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@xxxxxxxxxxxxx>
---
 src/rpc/virnetclient.c       | 30 ++++++++++++++++++------------
 src/rpc/virnetclientstream.c |  2 +-
 2 files changed, 19 insertions(+), 13 deletions(-)

diff --git a/src/rpc/virnetclient.c b/src/rpc/virnetclient.c
index 40d45b9..7aa5223 100644
--- a/src/rpc/virnetclient.c
+++ b/src/rpc/virnetclient.c
@@ -2216,20 +2216,26 @@ int virNetClientSendWithReplyStream(virNetClientPtr client,
                                     virNetMessagePtr msg,
                                     virNetClientStreamPtr st)
 {
-    int ret;
+    int ret = -1;
+
     virObjectLock(client);
-    /* Other thread might have already received
-     * stream EOF so we don't want sent anything.
-     * Server won't respond anyway.
-     */
-    if (virNetClientStreamEOF(st)) {
-        virObjectUnlock(client);
-        return 0;
+
+    if (virNetClientStreamRaiseError(st))
+        goto cleanup;
+
+    /* Check for EOF only if we are going to wait for incoming data */
+    if (!msg->bufferLength && virNetClientStreamEOF(st)) {
+        ret = 0;
+        goto cleanup;
     }
 
-    ret = virNetClientSendInternal(client, msg, true, false);
+    if (virNetClientSendInternal(client, msg, true, false) < 0)
+        goto cleanup;
+
+    ret = 0;
+
+ cleanup:
     virObjectUnlock(client);
-    if (ret < 0)
-        return -1;
-    return 0;
+
+    return ret;
 }
diff --git a/src/rpc/virnetclientstream.c b/src/rpc/virnetclientstream.c
index a17da31..3b0db52 100644
--- a/src/rpc/virnetclientstream.c
+++ b/src/rpc/virnetclientstream.c
@@ -350,7 +350,7 @@ int virNetClientStreamSendPacket(virNetClientStreamPtr st,
         if (virNetMessageEncodePayloadRaw(msg, NULL, 0) < 0)
             goto error;
 
-        if (virNetClientSendWithReply(client, msg) < 0)
+        if (virNetClientSendWithReplyStream(client, msg, st) < 0)
             goto error;
     }
 
-- 
1.8.3.1

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list