changeset 20197:dbe3a138adb1

dsync: race condition where done/finish is received after one side has closed We do not tell the remote we are closing if they have already told us because they close the connection after sending ITEM_FINISH or ITEM_DONE and will not be ever receive anything else from us unless it just happens to get combined into the same packet as a previous response and is already in the buffer.
author J. Nick Koston <nick@cpanel.net>
date Thu, 19 May 2016 19:15:49 -0500
parents ca35aefb6c0e
children 1779df80df17
files src/doveadm/dsync/dsync-ibc-stream.c
diffstat 1 files changed, 21 insertions(+), 4 deletions(-) [+]
line wrap: on
line diff
--- a/src/doveadm/dsync/dsync-ibc-stream.c	Tue May 24 15:52:37 2016 +0300
+++ b/src/doveadm/dsync/dsync-ibc-stream.c	Thu May 19 19:15:49 2016 -0500
@@ -166,6 +166,8 @@
 	unsigned int version_received:1;
 	unsigned int handshake_received:1;
 	unsigned int has_pending_data:1;
+	unsigned int finish_received:1;
+	unsigned int done_received:1;
 	unsigned int stopped:1;
 };
 
@@ -366,10 +368,22 @@
 	if (ibc->value_output != NULL)
 		i_stream_unref(&ibc->value_output);
 	else {
-		/* notify remote that we're closing. this is mainly to avoid
-		   "read() failed: EOF" errors on failing dsyncs */
-		o_stream_nsend_str(ibc->output,
-			t_strdup_printf("%c\n", items[ITEM_DONE].chr));
+		/* If the remote has not told us that they are closing we
+		   notify remote that we're closing. this is mainly to avoid
+		   "read() failed: EOF" errors on failing dsyncs.
+
+		   Avoid a race condition:
+		   We do not tell the remote we are closing if they have
+		   already told us because they close the
+		   connection after sending ITEM_DONE and will
+		   not be ever receive anything else from us unless
+		   it just happens to get combined into the same packet
+		   as a previous response and is already in the buffer.
+		*/
+		if (!ibc->done_received && !ibc->finish_received) {
+			o_stream_nsend_str(ibc->output,
+				t_strdup_printf("%c\n", items[ITEM_DONE].chr));
+		}
 		(void)o_stream_nfinish(ibc->output);
 	}
 
@@ -594,6 +608,7 @@
 		/* remote cleanly closed the connection, possibly because of
 		   some failure (which it should have logged). we don't want to
 		   log any stream errors anyway after this. */
+		ibc->done_received = TRUE;
 		dsync_ibc_stream_stop(ibc);
 		return DSYNC_IBC_RECV_RET_TRYAGAIN;
 
@@ -1939,6 +1954,8 @@
 		return DSYNC_IBC_RECV_RET_TRYAGAIN;
 	}
 	*mail_error_r = i;
+
+	ibc->finish_received = TRUE;
 	return DSYNC_IBC_RECV_RET_OK;
 }