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>

diff --git a/src/rpc/virnetclient.c b/src/rpc/virnetclient.c
index 40d45b9d2a819ecb94e418baa3acdcb71b4a648c..7aa52233ccc1a295b93c3d17c6c0cd81d92e9ef1 100644 (file)
--- 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 a17da313923c4b5618d6d005054eae5b8f3f6cfa..3b0db526d1ba0e2e7a5b59ebef7e18be534a6b0c 100644 (file)
--- 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;
     }