Discussion:
[Bug 790431] New: gstpad: Make calls to GstPadActivateFunction MT-safe
"GStreamer" (GNOME Bugzilla)
2017-11-16 07:29:20 UTC
Permalink
https://bugzilla.gnome.org/show_bug.cgi?id=790431

Bug ID: 790431
Summary: gstpad: Make calls to GstPadActivateFunction MT-safe
Classification: Platform
Product: GStreamer
Version: unspecified
OS: All
Status: NEW
Severity: normal
Priority: Normal
Component: gstreamer (core)
Assignee: gstreamer-***@lists.freedesktop.org
Reporter: ***@bilboed.com
QA Contact: gstreamer-***@lists.freedesktop.org
GNOME version: ---

See patch
--
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.
"GStreamer" (GNOME Bugzilla)
2017-11-16 07:29:26 UTC
Permalink
https://bugzilla.gnome.org/show_bug.cgi?id=790431

--- Comment #1 from Edward Hervey <***@bilboed.com> ---
Created attachment 363788
--> https://bugzilla.gnome.org/attachment.cgi?id=363788&action=edit
gstpad: Make calls to GstPadActivateFunction MT-safe

checking whether we already were in the target GstPadMode was being
done too early and there was the risk that we *would* end up
(de)activating a pad more than once.

Instead, re-do the check for pad mode when entering the final pad
(de)activation block.
--
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.
"GStreamer" (GNOME Bugzilla)
2017-11-16 09:07:23 UTC
Permalink
https://bugzilla.gnome.org/show_bug.cgi?id=790431

--- Comment #2 from Edward Hervey <***@bilboed.com> ---
Note that this commit does not solve the following problem:
* Thread 1 calls gst_pad_activate()
* Thread 2 was currently doing the (de)activation
* Thread 1 assumes pad (de)activation has happened and does something with it
while the (de)actvation hasn't *actually* completed.
--
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.
"GStreamer" (GNOME Bugzilla)
2017-11-16 09:20:37 UTC
Permalink
https://bugzilla.gnome.org/show_bug.cgi?id=790431

Sebastian Dröge (slomo) <***@coaxion.net> changed:

What |Removed |Added
----------------------------------------------------------------------------
CC| |***@coaxion.net
Attachment #363788|none |accepted-commit_now
status| |
--
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.
"GStreamer" (GNOME Bugzilla)
2017-11-16 09:55:41 UTC
Permalink
https://bugzilla.gnome.org/show_bug.cgi?id=790431

--- Comment #3 from Edward Hervey <***@bilboed.com> ---
Created attachment 363816
--> https://bugzilla.gnome.org/attachment.cgi?id=363816&action=edit
gstpad: Make pad (de)activation atomic

The following could happen previously:
* T1: calls gst_pad_set_active()
* T2: currently (de)activating it
* T1: gst_pad_set_active() returns, caller assumes that the pad has
completed the requested (de)activation ... whereas it is not
the case since the actual (de)activation in T2 might still be
going on.

To ensure atomicity of pad (de)activation, we use a internal
variable (and cond) to ensure only one thread at a time goes through
the actual (de)activation block
--
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.
"GStreamer" (GNOME Bugzilla)
2017-11-16 09:55:37 UTC
Permalink
https://bugzilla.gnome.org/show_bug.cgi?id=790431

Edward Hervey <***@bilboed.com> changed:

What |Removed |Added
----------------------------------------------------------------------------
CC| |***@bilboed.com
--
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.
"GStreamer" (GNOME Bugzilla)
2017-11-16 10:50:29 UTC
Permalink
https://bugzilla.gnome.org/show_bug.cgi?id=790431

Edward Hervey <***@bilboed.com> changed:

What |Removed |Added
----------------------------------------------------------------------------
Status|NEW |RESOLVED
Resolution|--- |FIXED

--- Comment #4 from Edward Hervey <***@bilboed.com> ---
commit d915dd4b20ffdcb417ea4b46a8dd9010c7ce7bf9 (HEAD -> master, origin/master,
origin/HEAD)
Author: Edward Hervey <***@centricular.com>
Date: Thu Nov 16 10:47:46 2017 +0100

gstpad: Make pad (de)activation atomic

The following could happen previously:
* T1: calls gst_pad_set_active()
* T2: currently (de)activating it
* T1: gst_pad_set_active() returns, caller assumes that the pad has
completed the requested (de)activation ... whereas it is not
the case since the actual (de)activation in T2 might still be
going on.

To ensure atomicity of pad (de)activation, we use a internal
variable (and cond) to ensure only one thread at a time goes through
the actual (de)activation block

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

commit 80262013ca553fa5b35e5c638893d3512a3dd7dc
Author: Edward Hervey <***@centricular.com>
Date: Thu Nov 16 08:26:12 2017 +0100

gstpad: Make calls to GstPadActivateFunction MT-safe

checking whether we already were in the target GstPadMode was being
done too early and there was the risk that we *would* end up
(de)activating a pad more than once.

Instead, re-do the check for pad mode when entering the final pad
(de)activation block.

https://bugzilla.gnome.org/show_bug.cgi?id=790431
--
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.
"GStreamer" (GNOME Bugzilla)
2017-11-16 10:50:42 UTC
Permalink
https://bugzilla.gnome.org/show_bug.cgi?id=790431

Edward Hervey <***@bilboed.com> changed:

What |Removed |Added
----------------------------------------------------------------------------
Attachment #363816|none |committed
status| |
--
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.
"GStreamer" (GNOME Bugzilla)
2017-11-16 10:50:47 UTC
Permalink
https://bugzilla.gnome.org/show_bug.cgi?id=790431

Edward Hervey <***@bilboed.com> changed:

What |Removed |Added
----------------------------------------------------------------------------
Attachment #363788|accepted-commit_now |committed
status| |
--
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.
"GStreamer" (GNOME Bugzilla)
2018-01-08 20:50:10 UTC
Permalink
https://bugzilla.gnome.org/show_bug.cgi?id=790431

Carlos Alberto Lopez Perez <***@igalia.com> changed:

What |Removed |Added
----------------------------------------------------------------------------
CC| |***@igalia.com

--- Comment #5 from Carlos Alberto Lopez Perez <***@igalia.com> ---
I have reported a deadlock on platform IMX that started to happen with WPE
(WebKit) after this two commits in bug 792341
--
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.
Loading...