After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 568394 - dropping the last reference to a stream filter closes the base stream
dropping the last reference to a stream filter closes the base stream
Status: RESOLVED FIXED
Product: glib
Classification: Platform
Component: gio
unspecified
Other All
: Normal normal
: ---
Assigned To: Allison Karlitskaya (desrt)
gtkdev
Depends on:
Blocks:
 
 
Reported: 2009-01-20 08:03 UTC by Allison Karlitskaya (desrt)
Modified: 2009-01-21 14:11 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
patch (25.33 KB, patch)
2009-01-20 21:49 UTC, Allison Karlitskaya (desrt)
needs-work Details | Review
updated patch (33.26 KB, patch)
2009-01-21 13:47 UTC, Allison Karlitskaya (desrt)
committed Details | Review

Description Allison Karlitskaya (desrt) 2009-01-20 08:03:31 UTC
consider this code:

main ()
{
  var client = new TcpClient ();
  var connection = client.connect_to_host ("mail.desrt.ca", "imap", null);
  var input = new DataInputStream (connection.get_input ());
  var output = connection.get_output ();

  print ("%s\n", input.read_line (null, null));
  output.write ("a starttls\n", 11, null);
  print ("%s\n", input.read_line (null, null));

  var tls_sess = new TLSSession (connection);
  var tls_con = tls_sess.handshake (null);

  input = new DataInputStream (tls_con.get_input ());
  output = tls_con.get_output ();

  output.write ("a capability\n", 13, null);
  print ("%s\n", input.read_line (null, null));
}

when input is set for the second time, vala calls g_object_unref on the original DataInputStream.  vala is holding the only reference, so the DataInputStream is finalized

in ginputstream.c we see that g_input_stream_finalize says:

  if (!stream->priv->closed)
    g_input_stream_close (stream, NULL, NULL);

and in gfilterinputstream.c we see in g_filter_input_stream_close:

  res = g_input_stream_close (base_stream,
                              cancellable,
                              error);

my original underlying input stream has now been closed.  the TLSSession wants to keep talking to that stream.
Comment 1 Allison Karlitskaya (desrt) 2009-01-20 08:53:45 UTC
alex notes that this is very much like the question of "should we close the fd?" when unix streams are closed.

he suggests:
03:48 < alex_> g_filter_input_stream_set_close_base()
03:48 < alex_> + get_close_base()


i'll write the patch for this later today.
Comment 2 Allison Karlitskaya (desrt) 2009-01-20 21:49:46 UTC
Created attachment 126874 [details] [review]
patch

2009-01-20  Ryan Lortie  <desrt@desrt.ca>

        * gfilterinputstream.h:
        * gfilterinputstream.c: add "close-base" property and only close
        * the
        base stream if it is true
        * gfilteroutputstream.h:
        * gfilteroutputstream.c: add a "close-base" property and only
        * close
        the base stream if it is true
        * gbufferedoutputstream (g_buffered_output_stream_close): check
        g_filter_output_stream_get_close_base() before closing the base
        stream.
        * gio.symbols: add
        * g_filter_{in,out}put_stream_{g,s}et_close_base
        * tests/filter-streams.c: new test case
        * tests/Makefile.am: add new test case
        * tests/.gitignore: add new test case
        * ../docs/reference/gio/gio-sections.txt: add new functions
---
 docs/reference/gio/gio-sections.txt |    4 +
 gio/ChangeLog                       |   20 +++++
 gio/gbufferedoutputstream.c         |   24 ++++---
 gio/gfilterinputstream.c            |  132 ++++++++++++++++++++++++++++-------
 gio/gfilterinputstream.h            |    4 +-
 gio/gfilteroutputstream.c           |  121 ++++++++++++++++++++++++++-----
 gio/gfilteroutputstream.h           |    4 +-
 gio/gio.symbols                     |    4 +
 gio/tests/.gitignore                |    1 +
 gio/tests/Makefile.am               |    4 +
 gio/tests/filter-streams.c          |   89 +++++++++++++++++++++++
 11 files changed, 350 insertions(+), 57 deletions(-)
 create mode 100644 gio/tests/filter-streams.c
Comment 3 Allison Karlitskaya (desrt) 2009-01-21 13:47:11 UTC
Created attachment 126918 [details] [review]
updated patch

2009-01-20  Ryan Lortie  <desrt@desrt.ca>

        * gfilterinputstream.h:
        * gfilterinputstream.c: add "close-base-stream" property and
        * only
        close the base stream if it is true.  issue async close callbacks from
        correct source object.
        * gfilteroutputstream.h:
        * gfilteroutputstream.c: add a "close-base-stream" property and
        * only
        close the base stream if it is true.  issue async close callbacks from
        correct source object.
        * gbufferedoutputstream: check
        * g_filter_output_stream_get_close_base()
        before closing the base stream.  fix invalid source tag comparison in
        close_async (was comparing to flush_async).
        * ../docs/reference/gio/gio-sections.txt:
        * gio.symbols: add
        g_filter_{in,out}put_stream_{g,s}et_close_base_stream
        * tests/filter-streams.c: new test cases
        * tests/Makefile.am: add new test
        * tests/.gitignore: add new test
---
 docs/reference/gio/gio-sections.txt |    4 +
 gio/ChangeLog                       |   23 ++++
 gio/gbufferedoutputstream.c         |   26 +++--
 gio/gfilterinputstream.c            |  153 ++++++++++++++++++----
 gio/gfilterinputstream.h            |    8 +-
 gio/gfilteroutputstream.c           |  146 ++++++++++++++++++----
 gio/gfilteroutputstream.h           |    8 +-
 gio/gio.symbols                     |    4 +
 gio/tests/.gitignore                |    1 +
 gio/tests/Makefile.am               |    4 +
 gio/tests/filter-streams.c          |  239 +++++++++++++++++++++++++++++++++++
 11 files changed, 551 insertions(+), 65 deletions(-)
 create mode 100644 gio/tests/filter-streams.c