GNOME Bugzilla – Bug 739983
tls: deal with gnutls's internal buffering of un-sent data
Last modified: 2018-05-29 06:39:43 UTC
Created attachment 290434 [details] [review] add flush support This comes from bug 739799. GStreamer sometimes doesn't resend the same data even after receiving G_IO_ERROR_WOULD_BLOCK. This is in gst_rtsp_watch_set_flushing. It then tries to send new data on the stream. However, GnuTLS is still expecting the same old data that couldn't be sent. This causes GnuTLS to return the old size instead of the new requested size (because it has sent the buffered data). Implementing the flush method would solve this problem as GStreamer could call g_output_stream_flush before trying to send any new data. May be this is all wrong and GStreamer should really send the old data instead of discarding it. From the commit log: GnuTLS buffers already encrypted data in gnutls_record_send when returning EAGAIN or EINTR. This means a user can call again gnutls_record_send with NULL and 0 and the previous data will be sent. This patch implements GOutputStream flush method by calling gnutls_record_send with NULL and 0 and thus making sure no data is buffered by GnuTLS.
As a side note. Patch for this bug and bug 739799 have been tested over ~300k connection/disconnection successfully.
Comment on attachment 290434 [details] [review] add flush support GTlsOutputStreamGnutls is violating the semantics of GOutputStream; if it claims to have not written the data, then it can't write it later without being asked to. GTlsConnectionGnutls needs to hide this behavior from higher levels somehow.
more generally, this patch doesn't really fix the problem, since there is no way that the caller can know for sure when it needs to call g_output_stream_flush() to fix the stream state
(In reply to comment #2) > (From update of attachment 290434 [details] [review]) > GTlsOutputStreamGnutls is violating the semantics of GOutputStream; if it > claims to have not written the data, then it can't write it later without being > asked to. GTlsConnectionGnutls needs to hide this behavior from higher levels > somehow. Yes, that's right. Whoever is responsible for handling the error should resend the data. This is not happening right now, and new data is being sent, which breaks everything. (In reply to comment #3) > more generally, this patch doesn't really fix the problem, since there is no > way that the caller can know for sure when it needs to call > g_output_stream_flush() to fix the stream state That's right too. It fixes my particular problem, but not the general one. However, glib documentation is not really clear about what to do. https://developer.gnome.org/gio/stable/GPollableOutputStream.html#g-pollable-output-stream-write-nonblocking Says: If stream is not currently writable, this will immediately return G_IO_ERROR_WOULD_BLOCK, and you can use g_pollable_output_stream_create_source() to create a GSource that will be triggered when stream is writable. But it doesn't say that you *must* write the same data again. Not doing it, in glib-networking gnutls case, breaks things.
(In reply to comment #4) > Yes, that's right. Whoever is responsible for handling the error should resend > the data. This is not happening right now, and new data is being sent, which > breaks everything. The fact that the "flush" works seems to suggest that whoever is on the other end of the connection does want/expect that data too, right? So you should be retrying? (Or is it just that things work out at the RTSP layer whether or not that data gets sent?) > If stream is not currently writable, this will immediately return > G_IO_ERROR_WOULD_BLOCK, and you can use > g_pollable_output_stream_create_source() to create a GSource that will be > triggered when stream is writable. > > But it doesn't say that you *must* write the same data again. Not doing it, in > glib-networking gnutls case, breaks things. You're not supposed to have to write the same data again. I'm not sure what the fix/workaround here is though...
(In reply to comment #5) > > The fact that the "flush" works seems to suggest that whoever is on the other > end of the connection does want/expect that data too, right? So you should be > retrying? (Or is it just that things work out at the RTSP layer whether or not > that data gets sent?) > Right, things keep working becasue this happens in the TEARDOWN request. So, the other end it's just waiting for the TEARDOWN response, if any. To put you in context. This problem only occurs when using RTSP interleaved data. That is, RTP data that is sent over the RTSP channel (so, over TCP). Then, both RTSP messages + data are sent on the same stream. Because we are sending quite some data over this channel, GnuTLS might return EAGAIN. This is OK before getting a TEARDOWN request, because the data not sent is retried (as expected). However, when a TEARDOWN request is received GStreamer discards data that has not been sent and as a final message it sends the TEARDOWN response. This is the one that makes everything go wrong, because of the GnuTLS buffering. > > You're not supposed to have to write the same data again. > > I'm not sure what the fix/workaround here is though... For the case described above, the solution would be, I guess, to force GStreamer send the data again. But, as you mentioned, it is still not clear what to do in the general case considering GnuTLS constraint. May be a new GError. Something like G_IO_ERROR_MUST_RESEND that would be returned for cases like GnuTLS. At least, this way, the user would know what to do before sending new data. Not sure if I like it much. Just a first idea.
(In reply to comment #6) > > May be a new GError. Something like G_IO_ERROR_MUST_RESEND that would be > returned for cases like GnuTLS. At least, this way, the user would know what to > do before sending new data. Not sure if I like it much. Just a first idea. This is not even backwards compatible. So it probably doesn't make any sense.
Geary is encountering something similar over in Bug 792219, except that this occurs when the underlying network connection has been dropped. I thought this was perhaps a bug in GBufferedOutputStream since it is attempting to flush a stream that is closed, but the underlying GTlsOutputStream is reporting that it is not yet closed, so GBufferedOutputStream would not be able to determine that it shouldn't be flushing the stream. From the stack trace in Bug 792219: > (gdb) up > #8 0x00007ffff7a7572a in flush_buffer_thread (task=0x55555a4c92c0 [GTask], object=0x55555b644f50, task_data=0x55555c089a50, cancellable=0x0) > at ../../../../gio/gbufferedoutputstream.c:664 > 664 in ../../../../gio/gbufferedoutputstream.c > (gdb) info locals > base_stream = 0x55555a530cc0 [GTlsOutputStreamGnutls] > fdata = 0x55555c089a50 > res = <optimised out> > error = 0x0 > (gdb) print base_stream.priv.closed > $3 = 0 >(gdb) print base_stream.priv.closing > $4 = 0
(In reply to Dan Winship from comment #5) > You're not supposed to have to write the same data again. > > I'm not sure what the fix/workaround here is though... I don't see any possible solution, other than to change GnuTLS to eliminate the requirement to resend the same data, or to document this limitation in GOutputStream.
(In reply to Michael Gratton from comment #8) > Geary is encountering something similar over in Bug 792219, except that this > occurs when the underlying network connection has been dropped. Your issue is unrelated.
-- GitLab Migration Automatic Message -- This bug has been migrated to GNOME's GitLab instance and has been closed from further activity. You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.gnome.org/GNOME/glib/issues/956.
Reopening. This bug was migrated to https://gitlab.gnome.org/GNOME/glib, but it should have been migrated to https://gitlab.gnome.org/GNOME/glib-networking.
-- GitLab Migration Automatic Message -- This bug has been migrated to GNOME's GitLab instance and has been closed from further activity. You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.gnome.org/GNOME/glib-networking/issues/15.