From a5445a3706b0bcbb6ec3f4f0f9e03b853ccf911b Mon Sep 17 00:00:00 2001
From: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com>
Date: Thu, 7 Feb 2019 15:58:39 +0300
Subject: [PATCH] rpc: fix race on stream abort/finish and server side abort

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@virtuozzo.com>
---
 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 40d45b9d2a..7aa52233cc 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 a17da31392..3b0db526d1 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;
     }