channel: Unqueue cancelled requests

We put a channel request on the output buffer and start writing, but
if the write is cancelled on the first call (i.e. no partial writes)
we abort immediately without ever writing the request.

However, if we do this we also need to unqueue the request from the output
buffer, as otherwise this will be sent with the next operation. This
can be problematic for seeks as the seek generation is then not in sync.

https://bugzilla.gnome.org/show_bug.cgi?id=675181

3 files changed, 42 insertions, 9 deletions

diff --git a/client/gdaemonfileinputstream.c b/client/gdaemonfileinputstream.c
index bfe02bde..f73c6816 100644
--- a/client/gdaemonfileinputstream.c
+++ b/client/gdaemonfileinputstream.c
@@ -394,6 +394,16 @@ error_is_cancel (GError *error)
 }
 
 static void
+unappend_request (GDaemonFileInputStream *stream)
+{
+  g_assert (stream->output_buffer->len >= G_VFS_DAEMON_SOCKET_PROTOCOL_REQUEST_SIZE);
+
+  stream->seq_nr--;
+  g_string_truncate (stream->output_buffer,
+		     stream->output_buffer->len - G_VFS_DAEMON_SOCKET_PROTOCOL_REQUEST_SIZE);
+}
+
+static void
 append_request (GDaemonFileInputStream *stream, guint32 command,
 		guint32 arg1, guint32 arg2, guint32 data_len,
 		guint32 *seq_nr)
@@ -623,6 +633,8 @@ iterate_read_state_machine (GDaemonFileInputStream *file, IOOperationData *io_op
 	case READ_STATE_WROTE_COMMAND:
 	  if (io_op->io_cancelled)
 	    {
+	      if (!op->sent_cancel)
+		unappend_request (file);
 	      op->ret_val = -1;
 	      g_set_error_literal (&op->ret_error,
 				   G_IO_ERROR,
@@ -894,6 +906,8 @@ iterate_close_state_machine (GDaemonFileInputStream *file, IOOperationData *io_o
 	case CLOSE_STATE_WROTE_REQUEST:
 	  if (io_op->io_cancelled)
 	    {
+	      if (!op->sent_cancel)
+		unappend_request (file);
 	      op->ret_val = FALSE;
 	      g_set_error_literal (&op->ret_error,
 				   G_IO_ERROR,
@@ -1142,6 +1156,8 @@ iterate_seek_state_machine (GDaemonFileInputStream *file, IOOperationData *io_op
 	case SEEK_STATE_WROTE_REQUEST:
 	  if (io_op->io_cancelled)
 	    {
+	      if (!op->sent_cancel)
+		unappend_request (file);
 	      op->ret_val = -1;
 	      g_set_error_literal (&op->ret_error,
 				   G_IO_ERROR,
@@ -1384,6 +1400,8 @@ iterate_query_state_machine (GDaemonFileInputStream *file,
 	case QUERY_STATE_WROTE_REQUEST:
 	  if (io_op->io_cancelled)
 	    {
+	      if (!op->sent_cancel)
+		unappend_request (file);
 	      op->info = NULL;
 	      g_set_error_literal (&op->ret_error,
 				   G_IO_ERROR,
diff --git a/client/gdaemonfileoutputstream.c b/client/gdaemonfileoutputstream.c
index 4516c975..70535b19 100644
--- a/client/gdaemonfileoutputstream.c
+++ b/client/gdaemonfileoutputstream.c
@@ -316,6 +316,15 @@ error_is_cancel (GError *error)
 }
 
 static void
+unappend_request (GDaemonFileOutputStream *stream)
+{
+  g_assert (stream->output_buffer->len >= G_VFS_DAEMON_SOCKET_PROTOCOL_REQUEST_SIZE);
+  stream->seq_nr--;
+  g_string_truncate (stream->output_buffer,
+		     stream->output_buffer->len - G_VFS_DAEMON_SOCKET_PROTOCOL_REQUEST_SIZE);
+}
+
+static void
 append_request (GDaemonFileOutputStream *stream, guint32 command,
 		guint32 arg1, guint32 arg2, guint32 data_len, guint32 *seq_nr)
 {
@@ -499,6 +508,8 @@ iterate_write_state_machine (GDaemonFileOutputStream *file, IOOperationData *io_
 	case WRITE_STATE_WROTE_COMMAND:
 	  if (io_op->io_cancelled)
 	    {
+	      if (!op->sent_cancel)
+		unappend_request (file);
 	      op->ret_val = -1;
 	      g_set_error_literal (&op->ret_error,
 				   G_IO_ERROR,
@@ -674,6 +685,8 @@ iterate_close_state_machine (GDaemonFileOutputStream *file, IOOperationData *io_
 	case CLOSE_STATE_WROTE_REQUEST:
 	  if (io_op->io_cancelled)
 	    {
+	      if (!op->sent_cancel)
+		unappend_request (file);
 	      op->ret_val = FALSE;
 	      g_set_error_literal (&op->ret_error,
 				   G_IO_ERROR,
@@ -866,6 +879,8 @@ iterate_seek_state_machine (GDaemonFileOutputStream *file, IOOperationData *io_o
 	case SEEK_STATE_WROTE_REQUEST:
 	  if (io_op->io_cancelled)
 	    {
+	      if (!op->sent_cancel)
+		unappend_request (file);
 	      op->ret_val = -1;
 	      g_set_error_literal (&op->ret_error,
 				   G_IO_ERROR,
@@ -1045,6 +1060,8 @@ iterate_query_state_machine (GDaemonFileOutputStream *file,
 	case QUERY_STATE_WROTE_REQUEST:
 	  if (io_op->io_cancelled)
 	    {
+	      if (!op->sent_cancel)
+		unappend_request (file);
 	      op->info = NULL;
 	      g_set_error_literal (&op->ret_error,
 				   G_IO_ERROR,
diff --git a/daemon/gvfschannel.c b/daemon/gvfschannel.c
index ec87cb99..994b5ea1 100644
--- a/daemon/gvfschannel.c
+++ b/daemon/gvfschannel.c
@@ -321,21 +321,19 @@ start_queued_request (GVfsChannel *channel)
 				   req->arg1, req->arg2,
 				   req->data, req->data_len,
 				   &error);
-      if (job == NULL)
-	{
-	  job = g_vfs_job_error_new (channel, error);
-	  g_error_free (error);
-	}
-
 
       if (job != NULL && req->cancelled)
 	{
 	  /* Ignore the job, although we need to create it to rely
 	     on handle_request side effects like seek generations, etc */
 	  g_object_unref (job);
-	  error =
-	    g_error_new_literal (G_IO_ERROR, G_IO_ERROR_CANCELLED,
-				 _("Operation was cancelled"));
+	  job = NULL;
+	  error =  g_error_new_literal (G_IO_ERROR, G_IO_ERROR_CANCELLED,
+					_("Operation was cancelled"));
+	}
+
+      if (job == NULL)
+	{
 	  job = g_vfs_job_error_new (channel, error);
 	  g_error_free (error);
 	}