[glib-networking] Input/output streams should expect to outlast their GTlsConnection

[Michael Catanzaro <mcatanzaro src gnome org>]

commit e5dac2ba5f21fc53f1461992224cf56e4a345cdb
Author: Michael Catanzaro <mcatanzaro igalia com>
Date:   Thu Mar 1 15:31:37 2018 -0600

    Input/output streams should expect to outlast their GTlsConnection
    
    While it's not great for the GTlsConnection's GInputStream and
    GOutputStream objects to outlive the GTlsConnection, it's hardly
    unexpected: reffing objects is hardly disallowed. We should return nice
    errors in this case instead of emitting criticals.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=792219

 tls/gnutls/gtlsinputstream-gnutls.c  |   29 ++++++++++++++++-----
 tls/gnutls/gtlsoutputstream-gnutls.c |   30 ++++++++++++++++------
 tls/tests/connection.c               |   46 ++++++++++++++++++++++++++++++++++
 3 files changed, 90 insertions(+), 15 deletions(-)
---
diff --git a/tls/gnutls/gtlsinputstream-gnutls.c b/tls/gnutls/gtlsinputstream-gnutls.c
index 99f0f85..9128480 100644
--- a/tls/gnutls/gtlsinputstream-gnutls.c
+++ b/tls/gnutls/gtlsinputstream-gnutls.c
@@ -25,6 +25,8 @@
 #include "config.h"
 #include "gtlsinputstream-gnutls.h"
 
+#include <glib/gi18n.h>
+
 struct _GTlsInputStreamGnutls
 {
   GInputStream parent_instance;
@@ -70,7 +72,12 @@ g_tls_input_stream_gnutls_read (GInputStream  *stream,
   gssize ret;
 
   conn = g_weak_ref_get (&tls_stream->weak_conn);
-  g_return_val_if_fail (conn != NULL, -1);
+  if (conn == NULL)
+    {
+      g_set_error_literal (error, G_IO_ERROR, G_IO_ERROR_CLOSED,
+                           _("Connection is closed"));
+      return -1;
+    }
 
   ret = g_tls_connection_gnutls_read (conn,
                                       buffer, count, -1  /* blocking */,
@@ -87,7 +94,8 @@ g_tls_input_stream_gnutls_pollable_is_readable (GPollableInputStream *pollable)
   gboolean ret;
 
   conn = g_weak_ref_get (&tls_stream->weak_conn);
-  g_return_val_if_fail (conn != NULL, FALSE);
+  if (conn == NULL)
+    return FALSE;
 
   ret = g_tls_connection_gnutls_check (conn, G_IO_IN);
 
@@ -104,7 +112,12 @@ g_tls_input_stream_gnutls_pollable_create_source (GPollableInputStream *pollable
   GSource *ret;
 
   conn = g_weak_ref_get (&tls_stream->weak_conn);
-  g_return_val_if_fail (conn != NULL, NULL);
+  if (conn == NULL)
+    {
+      ret = g_idle_source_new ();
+      g_source_set_name (ret, "[glib-networking] g_tls_input_stream_gnutls_pollable_create_source dummy 
source");
+      return ret;
+    }
 
   ret = g_tls_connection_gnutls_create_source (conn, G_IO_IN, cancellable);
   g_object_unref (conn);
@@ -122,7 +135,12 @@ g_tls_input_stream_gnutls_pollable_read_nonblocking (GPollableInputStream  *poll
   gssize ret;
 
   conn = g_weak_ref_get (&tls_stream->weak_conn);
-  g_return_val_if_fail (conn != NULL, -1);
+  if (conn == NULL)
+    {
+      g_set_error_literal (error, G_IO_ERROR, G_IO_ERROR_CLOSED,
+                           _("Connection is closed"));
+      return -1;
+    }
 
   ret = g_tls_connection_gnutls_read (conn, buffer, size,
                                       0  /* non-blocking */, NULL, error);
@@ -142,9 +160,6 @@ g_tls_input_stream_gnutls_close (GInputStream            *stream,
 
   conn = g_weak_ref_get (&tls_stream->weak_conn);
 
-  /* Special case here because this is called by the finalize
-   * of the main GTlsConnection object.
-   */
   if (conn == NULL)
     return TRUE;
 
diff --git a/tls/gnutls/gtlsoutputstream-gnutls.c b/tls/gnutls/gtlsoutputstream-gnutls.c
index 44b10f7..062b8ef 100644
--- a/tls/gnutls/gtlsoutputstream-gnutls.c
+++ b/tls/gnutls/gtlsoutputstream-gnutls.c
@@ -25,6 +25,8 @@
 #include "config.h"
 #include "gtlsoutputstream-gnutls.h"
 
+#include <glib/gi18n.h>
+
 struct _GTlsOutputStreamGnutls
 {
   GOutputStream parent_instance;
@@ -70,7 +72,12 @@ g_tls_output_stream_gnutls_write (GOutputStream  *stream,
   gssize ret;
 
   conn = g_weak_ref_get (&tls_stream->weak_conn);
-  g_return_val_if_fail (conn != NULL, -1);
+  if (conn == NULL)
+    {
+      g_set_error_literal (error, G_IO_ERROR, G_IO_ERROR_CLOSED,
+                           _("Connection is closed"));
+      return -1;
+    }
 
   ret = g_tls_connection_gnutls_write (conn, buffer, count, -1  /* blocking */,
                                        cancellable, error);
@@ -86,7 +93,8 @@ g_tls_output_stream_gnutls_pollable_is_writable (GPollableOutputStream *pollable
   gboolean ret;
 
   conn = g_weak_ref_get (&tls_stream->weak_conn);
-  g_return_val_if_fail (conn != NULL, FALSE);
+  if (conn == NULL)
+    return FALSE;
 
   ret = g_tls_connection_gnutls_check (conn, G_IO_OUT);
 
@@ -104,7 +112,12 @@ g_tls_output_stream_gnutls_pollable_create_source (GPollableOutputStream *pollab
   GSource *ret;
 
   conn = g_weak_ref_get (&tls_stream->weak_conn);
-  g_return_val_if_fail (conn != NULL, NULL);
+  if (conn == NULL)
+    {
+      ret = g_idle_source_new ();
+      g_source_set_name (ret, "[glib-networking] g_tls_output_stream_gnutls_pollable_create_source dummy 
source");
+      return ret;
+    }
 
   ret = g_tls_connection_gnutls_create_source (conn,
                                                G_IO_OUT,
@@ -124,7 +137,12 @@ g_tls_output_stream_gnutls_pollable_write_nonblocking (GPollableOutputStream  *p
   gssize ret;
 
   conn = g_weak_ref_get (&tls_stream->weak_conn);
-  g_return_val_if_fail (conn != NULL, -1);
+  if (conn == NULL)
+    {
+      g_set_error_literal (error, G_IO_ERROR, G_IO_ERROR_CLOSED,
+                           _("Connection is closed"));
+      return -1;
+    }
 
   ret = g_tls_connection_gnutls_write (conn, buffer, size,
                                        0  /* non-blocking */, NULL, error);
@@ -143,10 +161,6 @@ g_tls_output_stream_gnutls_close (GOutputStream            *stream,
   gboolean ret;
 
   conn = g_weak_ref_get (&tls_stream->weak_conn);
-
-  /* Special case here because this is called by the finalize
-   * of the main GTlsConnection object.
-   */
   if (conn == NULL)
     return TRUE;
 
diff --git a/tls/tests/connection.c b/tls/tests/connection.c
index bf9e154..2c59001 100644
--- a/tls/tests/connection.c
+++ b/tls/tests/connection.c
@@ -2111,6 +2111,50 @@ test_garbage_database (TestConnection *test,
   g_assert_no_error (test->server_error);
 }
 
+static void
+test_readwrite_after_connection_destroyed (TestConnection *test,
+                                           gconstpointer   data)
+{
+  GIOStream *connection;
+  GOutputStream *ostream;
+  GInputStream *istream;
+  unsigned char buffer[1];
+  GError *error = NULL;
+
+  g_test_bug ("792219");
+
+  connection = start_async_server_and_connect_to_it (test, G_TLS_AUTHENTICATION_REQUESTED, TRUE);
+  test->client_connection = g_tls_client_connection_new (connection, test->identity, &error);
+  g_assert_no_error (error);
+  g_object_unref (connection);
+
+  istream = g_object_ref (g_io_stream_get_input_stream (test->client_connection));
+  ostream = g_object_ref (g_io_stream_get_output_stream (test->client_connection));
+  g_clear_object (&test->client_connection);
+
+  /* The GTlsConnection has been destroyed, but its underlying streams
+   * live on, because we have reffed them. Verify that attempts to read
+   * and write produce only nice GErrors.
+   */
+  g_input_stream_read (istream, buffer, sizeof (buffer), NULL, &error);
+  g_assert_error (error, G_IO_ERROR, G_IO_ERROR_CLOSED);
+  g_clear_error (&error);
+
+  g_output_stream_write (ostream, TEST_DATA, TEST_DATA_LENGTH,
+                         G_PRIORITY_DEFAULT, &error);
+  g_assert_error (error, G_IO_ERROR, G_IO_ERROR_CLOSED);
+  g_clear_error (&error);
+
+  g_input_stream_close (istream, NULL, &error);
+  g_assert_no_error (error);
+
+  g_output_stream_close (ostream, NULL, &error);
+  g_assert_no_error (error);
+
+  g_object_unref (istream);
+  g_object_unref (ostream);
+}
+
 int
 main (int   argc,
       char *argv[])
@@ -2186,6 +2230,8 @@ main (int   argc,
               setup_connection, test_fallback, teardown_connection);
   g_test_add ("/tls/connection/garbage-database", TestConnection, NULL,
               setup_connection, test_garbage_database, teardown_connection);
+  g_test_add ("/tls/connection/readwrite-after-connection-destroyed", TestConnection, NULL,
+              setup_connection, test_readwrite_after_connection_destroyed, teardown_connection);
 
   ret = g_test_run ();