GNOME Bugzilla – Bug 728960
curl*sink: Deliver errors from transfer thread to stream thread in a better way
Last modified: 2018-01-25 13:10:46 UTC
The reason for these changes is that previously if an error occurred in the transfer-thread it would call GST_ELEMENT_ERROR() to signal on the pipeline bus that an error had occurred. When doing so it tried to acquire the lock of the curlsink's parent which may already be held in the context of the stream thread. With these changes you will now have the transfer thread signal errors back to the stream thread and the stream thread will then call GST_ELEMENT_ERROR(). This avoids the deadlock. In addition you get a series of cleanup patches.
Created attachment 275143 [details] [review] Proposed patch [1/7]
Created attachment 275144 [details] [review] Proposed patch [2/7]
Created attachment 275145 [details] [review] Proposed patch [3/7]
Created attachment 275146 [details] [review] Proposed patch [4/7]
Created attachment 275147 [details] [review] Proposed patch [5/7]
Created attachment 275148 [details] [review] Proposed patch [6/7]
Created attachment 275149 [details] [review] Proposed patch [7/7]
Comment on attachment 275149 [details] [review] Proposed patch [7/7] This succeeds just fine before and after your changes :) Just that it prints a GST_ERROR() after your changes.
commit 241c3acad52f5091836f60e4678f910ae28113b5 Author: Sebastian Rasmussen <sebrn@axis.com> Date: Wed Apr 16 01:50:16 2014 +0200 curl*sink: report errors from curl when setting options https://bugzilla.gnome.org/show_bug.cgi?id=728960 commit c75c7a9a53e1d2e34ba96600923a294651db7f89 Author: Sebastian Rasmussen <sebrn@axis.com> Date: Tue Apr 15 23:53:32 2014 +0200 curl*sink: post error on bus in element, not transfer thread https://bugzilla.gnome.org/show_bug.cgi?id=728960 commit 13f87a1db19c137d5c31bec216472aee6bbcb299 Author: Sebastian Rasmussen <sebrn@axis.com> Date: Tue Apr 22 22:26:32 2014 +0200 curlhttpsink: allow for unconditionally disabling proxying Previously if the proxy server hostname was the empty string curlhttpsink would never even set the libcurl option. For libcurl however, having a proxy server hostname be the empty string means that proxying should be disabled even if environment variables might be set. Now with the restriction lifted, doing this is allowed. https://bugzilla.gnome.org/show_bug.cgi?id=728960 commit 54d160be5f613f2e497f61b99339d6565b32c174 Author: Sebastian Rasmussen <sebrn@axis.com> Date: Tue Apr 22 22:26:56 2014 +0200 curl*sink: improve return value checks in test cases https://bugzilla.gnome.org/show_bug.cgi?id=728960 commit da987a3219805f7f1c62d554ba528e6966be68fb Author: Sebastian Rasmussen <sebrn@axis.com> Date: Tue Apr 22 22:26:12 2014 +0200 curl*sink: fix typos https://bugzilla.gnome.org/show_bug.cgi?id=728960 commit bdd2676a67fa2fb3c6c48d332f6e398a877cb61e Author: Sebastian Rasmussen <sebrn@axis.com> Date: Wed Apr 16 01:50:03 2014 +0200 curl*sink: fix some gst-indent problems https://bugzilla.gnome.org/show_bug.cgi?id=728960
Now I can explain my changes in curl*sink better! :) Basically what I saw was that I was in the process of changing state on the pipeline when the curl*sink encountered an error condition. When it did so it attempted to call gst_element_message_full() as part of GST_ELEMENT_ERROR(). This attempts to take the parent lock of the element (i.e. the pipeline), but of course this lock was already taken somewhere in the state changing part. I'll try to fix my testcase to expose this behaviour.
Sebastian (sebras): what's up with the last patch? Can this be closed? Do you plan on finishing it up? Do you need further input?