Package: glib2.0 / 2.42.1-1

gdbus-Let-the-pending-read-finish-before-closing-the.patch Patch series | download
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168
169
170
171
172
173
174
175
176
177
178
179
180
181
182
183
184
185
186
187
188
189
190
191
From: Mikhail Zabaluev <mikhail.zabaluev@gmail.com>
Date: Mon, 3 Feb 2014 02:16:53 +0200
Subject: [PATCH] gdbus: Let the pending read finish before closing the
 connection

This prevents the GIOStream from being closed with a read pending
on the input substream. g_io_stream_close_async() is actually
supposed to fail in this case; see
https://bugzilla.gnome.org/show_bug.cgi?id=707912

Bug: https://bugzilla.gnome.org/show_bug.cgi?id=723719
Bug-Debian: http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=738290
Reviewed-by: Simon McVittie <simon.mcvittie@collabora.co.uk>
Applied-upstream: no
---
 gio/gdbusprivate.c | 75 +++++++++++++++++++++++++++---------------------------
 1 file changed, 38 insertions(+), 37 deletions(-)

--- a/gio/gdbusprivate.c
+++ b/gio/gdbusprivate.c
@@ -335,6 +335,8 @@
     PENDING_CLOSE
 } OutputPending;
 
+static void continue_writing (GDBusWorker *worker);
+
 struct GDBusWorker
 {
   volatile gint                       ref_count;
@@ -577,7 +579,10 @@
 
   /* If already stopped, don't even process the reply */
   if (g_atomic_int_get (&worker->stopped))
-    goto out;
+    {
+      worker->close_expected = TRUE;
+      goto out;
+    }
 
   error = NULL;
   if (worker->socket == NULL)
@@ -672,21 +677,16 @@
        * if the GDBusConnection tells us to close (either via
        * _g_dbus_worker_stop, which is called on last-unref, or directly),
        * so a cancelled read must mean our connection was closed locally.
-       *
-       * If we're closing, other errors are possible - notably,
-       * G_IO_ERROR_CLOSED can be seen if we close the stream with an async
-       * read in-flight. It seems sensible to treat all read errors during
-       * closing as an expected thing that doesn't trip exit-on-close.
-       *
-       * Because close_expected can't be set until we get into the worker
-       * thread, but the cancellable is signalled sooner (from another
-       * thread), we do still need to check the error.
        */
-      if (worker->close_expected ||
-          g_error_matches (error, G_IO_ERROR, G_IO_ERROR_CANCELLED))
-        _g_dbus_worker_emit_disconnected (worker, FALSE, NULL);
+      if (g_error_matches (error, G_IO_ERROR, G_IO_ERROR_CANCELLED))
+        {
+          _g_dbus_worker_emit_disconnected (worker, FALSE, NULL);
+          worker->close_expected = TRUE;
+        }
       else
-        _g_dbus_worker_emit_disconnected (worker, TRUE, error);
+        {
+          _g_dbus_worker_emit_disconnected (worker, TRUE, error);
+        }
 
       g_error_free (error);
       goto out;
@@ -815,6 +815,13 @@
  out:
   g_mutex_unlock (&worker->read_lock);
 
+  /* If need to close the stream, make sure the output logic runs its course.
+   * Because this is the worker thread, we can read these struct members
+   * without holding the lock: no other thread ever modifies them.
+   */
+  if (worker->close_expected && worker->output_pending == PENDING_NONE)
+    continue_writing (worker);
+
   /* gives up the reference acquired when calling g_input_stream_read_async() */
   _g_dbus_worker_unref (worker);
 }
@@ -823,8 +830,8 @@
 static void
 _g_dbus_worker_do_read_unlocked (GDBusWorker *worker)
 {
-  /* Note that we do need to keep trying to read even if close_expected is
-   * true, because only failing a read causes us to signal 'closed'.
+  /* We need to keep trying to read, because only failing a read
+   * causes us to signal 'closed'.
    */
 
   /* if bytes_wanted is zero, it means start reading a message */
@@ -1145,8 +1152,6 @@
 }
 /* ---------------------------------------------------------------------------------------------------- */
 
-static void continue_writing (GDBusWorker *worker);
-
 typedef struct
 {
   GDBusWorker *worker;
@@ -1445,12 +1450,15 @@
   /* if we want to close the connection, that takes precedence */
   if (worker->pending_close_attempts != NULL)
     {
-      worker->close_expected = TRUE;
-      worker->output_pending = PENDING_CLOSE;
+      /* close only once the read is finished */
+      if (worker->close_expected)
+        {
+          worker->output_pending = PENDING_CLOSE;
 
-      g_io_stream_close_async (worker->stream, G_PRIORITY_DEFAULT,
-                               NULL, iostream_close_cb,
-                               _g_dbus_worker_ref (worker));
+          g_io_stream_close_async (worker->stream, G_PRIORITY_DEFAULT,
+                                   NULL, iostream_close_cb,
+                                   _g_dbus_worker_ref (worker));
+        }
     }
   else
     {
@@ -1558,7 +1566,6 @@
 /**
  * @write_data: (transfer full) (allow-none):
  * @flush_data: (transfer full) (allow-none):
- * @close_data: (transfer full) (allow-none):
  *
  * Can be called from any thread
  *
@@ -1568,8 +1575,7 @@
 static void
 schedule_writing_unlocked (GDBusWorker        *worker,
                            MessageToWriteData *write_data,
-                           FlushData          *flush_data,
-                           CloseData          *close_data)
+                           FlushData          *flush_data)
 {
   if (write_data != NULL)
     g_queue_push_tail (worker->write_queue, write_data);
@@ -1577,10 +1583,6 @@
   if (flush_data != NULL)
     worker->write_pending_flushes = g_list_prepend (worker->write_pending_flushes, flush_data);
 
-  if (close_data != NULL)
-    worker->pending_close_attempts = g_list_prepend (worker->pending_close_attempts,
-                                                     close_data);
-
   /* If we had output pending, the next bit of output will happen
    * automatically when it finishes, so we only need to do this
    * if nothing was pending.
@@ -1629,7 +1631,7 @@
   data->blob_size = blob_len;
 
   g_mutex_lock (&worker->write_lock);
-  schedule_writing_unlocked (worker, data, NULL, NULL);
+  schedule_writing_unlocked (worker, data, NULL);
   g_mutex_unlock (&worker->write_lock);
 }
 
@@ -1710,13 +1712,12 @@
       (cancellable == NULL ? NULL : g_object_ref (cancellable));
   close_data->result = (result == NULL ? NULL : g_object_ref (result));
 
-  /* Don't set worker->close_expected here - we're in the wrong thread.
-   * It'll be set before the actual close happens.
-   */
-  g_cancellable_cancel (worker->cancellable);
   g_mutex_lock (&worker->write_lock);
-  schedule_writing_unlocked (worker, NULL, NULL, close_data);
+  worker->pending_close_attempts = g_list_prepend (worker->pending_close_attempts,
+                                                   close_data);
   g_mutex_unlock (&worker->write_lock);
+
+  g_cancellable_cancel (worker->cancellable);
 }
 
 /* This can be called from any thread - frees worker. Note that
@@ -1786,7 +1787,7 @@
       data->number_to_wait_for = worker->write_num_messages_written + pending_writes;
       g_mutex_lock (&data->mutex);
 
-      schedule_writing_unlocked (worker, NULL, data, NULL);
+      schedule_writing_unlocked (worker, NULL, data);
     }
   g_mutex_unlock (&worker->write_lock);