[geary/mjog/imap-connection-fixes: 24/34] Geary.Imap.Serialiser: Stop using GDataOutputStream



commit 734745133827eb957c907d20cb797cc2b23d0c68
Author: Michael Gratton <mike vee net>
Date:   Fri Mar 13 09:57:15 2020 +1100

    Geary.Imap.Serialiser: Stop using GDataOutputStream
    
    We're getting another report of segfaults from GDataOutputStream's
    internal thread (in addition to the issue that commit 479e9946 was
    workgin around), so just stop using it.
    
    By using a buffered stream and writing directly to it, we won't block
    the main thread except for commands with very large parameter sets,
    which shouldn't happen too often.

 src/engine/imap/transport/imap-serializer.vala | 50 +++++++++++++++-----------
 1 file changed, 29 insertions(+), 21 deletions(-)
---
diff --git a/src/engine/imap/transport/imap-serializer.vala b/src/engine/imap/transport/imap-serializer.vala
index f1a6a4fc..77641b06 100644
--- a/src/engine/imap/transport/imap-serializer.vala
+++ b/src/engine/imap/transport/imap-serializer.vala
@@ -1,34 +1,38 @@
 /*
- * Copyright 2016 Software Freedom Conservancy Inc.
- * Copyright 2018 Michael Gratton <mike vee net>
+ * Copyright © 2016 Software Freedom Conservancy Inc.
+ * Copyright © 2018, 2020 Michael Gratton <mike vee net>
  *
  * This software is licensed under the GNU Lesser General Public License
  * (version 2.1 or later).  See the COPYING file in this distribution.
  */
 
 /**
- * Writes IMAP protocol strings to a supplied output stream.
+ * Writes IMAP protocol strings to the supplied output stream.
  *
- * This class uses a {@link GLib.DataOutputStream} for writing strings
- * to the given stream. Since that does not support asynchronous
- * writes, it is highly desirable that the stream passed to this class
- * is a {@link GLib.BufferedOutputStream}, or some other type that
- * uses a memory buffer large enough to write a typical command
- * completely without causing disk or network I/O.
+ * Since most IMAP commands are small (with the exception of literal
+ * data) this class writes directly, synchronously to the given
+ * stream. Thus it is highly desirable that the stream passed to the
+ * constructor is buffered, either a {@link
+ * GLib.BufferedOutputStream}, or some other type that uses a memory
+ * buffer large enough to write a typical command completely without
+ * causing disk or network I/O.
  *
  * @see Deserializer
  */
 public class Geary.Imap.Serializer : BaseObject {
 
 
+    private const string EOL = "\r\n";
+    private const string SPACE = " ";
+
     private string identifier;
-    private GLib.DataOutputStream output;
+    private GLib.OutputStream output;
+
 
 
     public Serializer(string identifier, GLib.OutputStream output) {
         this.identifier = identifier;
-        this.output = new GLib.DataOutputStream(output);
-        this.output.set_close_base_stream(false);
+        this.output = output;
     }
 
     /**
@@ -41,7 +45,7 @@ public class Geary.Imap.Serializer : BaseObject {
     public void push_unquoted_string(string str,
                                      GLib.Cancellable? cancellable = null)
         throws GLib.Error {
-        this.output.put_string(str, cancellable);
+        this.output.write_all(str.data, null, cancellable);
     }
 
     /**
@@ -54,17 +58,19 @@ public class Geary.Imap.Serializer : BaseObject {
     public void push_quoted_string(string str,
                                    GLib.Cancellable? cancellable = null)
         throws GLib.Error {
-        this.output.put_byte('"');
+        StringBuilder buf = new StringBuilder.sized(str.length + 2);
+        buf.append_c('"');
         int index = 0;
         char ch = str[index];
         while (ch != String.EOS) {
             if (ch == '"' || ch == '\\') {
-                this.output.put_byte('\\');
+                buf.append_c('\\');
             }
-            this.output.put_byte(ch);
+            buf.append_c(ch);
             ch = str[++index];
         }
-        this.output.put_byte('"');
+        buf.append_c('"');
+        this.output.write_all(buf.data, null, cancellable);
     }
 
     /**
@@ -75,7 +81,9 @@ public class Geary.Imap.Serializer : BaseObject {
      */
     public void push_ascii(char ch, GLib.Cancellable? cancellable = null)
         throws GLib.Error {
-        this.output.put_byte(ch, cancellable);
+        // allocate array on the stack to avoid mem alloc overhead
+        uint8 buf[1] = { ch };
+        this.output.write_all(buf, null, cancellable);
     }
 
     /**
@@ -83,7 +91,7 @@ public class Geary.Imap.Serializer : BaseObject {
      */
     public void push_space(GLib.Cancellable? cancellable = null)
         throws GLib.Error {
-        this.output.put_byte(' ', cancellable);
+        this.output.write_all(SPACE.data, null, cancellable);
     }
 
     /**
@@ -91,7 +99,7 @@ public class Geary.Imap.Serializer : BaseObject {
      */
     public void push_nil(GLib.Cancellable? cancellable = null)
         throws GLib.Error {
-        this.output.put_string(NilParameter.VALUE, cancellable);
+        this.output.write_all(NilParameter.VALUE.data, null, cancellable);
     }
 
     /**
@@ -99,7 +107,7 @@ public class Geary.Imap.Serializer : BaseObject {
      */
     public void push_eol(GLib.Cancellable? cancellable = null)
         throws GLib.Error {
-        this.output.put_string("\r\n", cancellable);
+        this.output.write_all(EOL.data, null, cancellable);
     }
 
     /**


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]