[Bug 767501] curlsmtpsink: Lock and don't send final boundary upon error

[GStreamer (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=767501

Tim-Philipp Müller <t.i.m at zen.co.uk> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
                 CC|                            |t.i.m at zen.co.uk
         Resolution|---                         |FIXED
   Target Milestone|git master                  |1.9.1

--- Comment #2 from Tim-Philipp Müller <t.i.m at zen.co.uk> ---
Pushed, thanks (after fixing up indentation, please install gnu indent):

commit c7e421712155cc01b42bee9adea253134ceea5c1
Author: Sebastian Rasmussen <sebrn at axis.com>
Date:   Thu Jun 9 01:20:36 2016 +0200

    curlsmtpsink: Lock and don't send final boundary upon error

    Previously GstCurlSmtpSink could cause the pipeline thread to end up
    waiting for a stopped thread to perform work.

    The scenario was that the sink could be rendering a buffer and waiting
    for the curl transfer thread to have sent the data. As soon as the
    transfer thread has copied all data to curl's data buffer in
    gst_curl_base_sink_transfer_read_cb() then the render call would stop
    waiting and return GST_FLOW_OK. While this takes place the transfer
    thread may suffer from an error e.g. due gst_poll_wait() timing out.
    This causes the transfer thread to record the error, claim (it is not
    really true since there was an error) that the data has been sent and
    that a response has been received by trying to signal the pipeline
    thread (but this has already stopped waiting). Finally the transfer
    thread stops itself. A short while later the pipeline thread may attempt
    to push an EOS event into GstCurlSmtpSink. Since there is no check in
    gst_curl_smtp_sink_event() to check if the sink has suffered from any
    error it may attempt to add a final boundary and ask the, now deceased,
    transfer thread to transfer the new data. Next the sink element would
    have waited for the transfer to complete (using a different mechanism
    than normal transfers through GstCurlBaseSink). In this case there was
    an error check to avoid waiting if an error had already been seen.
    Finally GstCurlSmtpSink would chain up to GstCurlBaseSink which would
    then block waiting for a response (normally this would be prevented by
    the transfer thread suffering the error claiming that it had been
    received, but GstCurlSmtpSink clobbered this flag after the fact).

    Now GstCurlSmtpSink avoids this by locking over the entire event handing
    (preventing simultaneous changes to flags by the two threads) and also
    by avoiding to initiate transfer of final boundary if an error has
    already been seen.

    Also add GST_FIXME() for remaining similar issue where the pipeline
    thread may block indefinitely waiting for transfer thread to transfer
    data but the transfer thread errors out and fails to notify the pipeline
    thread that the transfer failed.

    https://bugzilla.gnome.org/show_bug.cgi?id=767501

-- 
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.