mirrors/gstreamer
mirrors/gstreamer
1
1
Fork 0
mirror of https://gitlab.freedesktop.org/gstreamer/gstreamer.git synced 2024-12-29 11:40:38 +00:00
Code Issues 1 Projects Releases Wiki Activity

omx: Fix another race condition in the recursive mutex

Browse source 
Between lock() and begin_recursion() it was possible for another thread to
try to do a recursive_lock(). This would block because the mutex was already
locked(), but not ready for recursive locking yet. unlock() would never
happen in the original thread because it was waiting for the other thread
to finish first.

Happened on the Raspberry Pi.
This commit is contained in:
Sebastian Dröge 2012-12-20 17:46:36 +01:00
parent 1c7fcf832e
commit e026926951
3 changed files with 67 additions and 17 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

29
omx/gstomx.c
View file

@ -538,7 +538,7 @@ gst_omx_component_set_state (GstOMXComponent * comp, OMX_STATETYPE state)
g_return_val_if_fail (comp != NULL, OMX_ErrorUndefined);
gst_omx_rec_mutex_lock (&comp->state_lock);
gst_omx_rec_mutex_lock_for_recursion (&comp->state_lock);
old_state = comp->state;
GST_DEBUG_OBJECT (comp->parent, "Setting state from %d to %d", old_state,
state);
@ -570,7 +570,7 @@ gst_omx_component_set_state (GstOMXComponent * comp, OMX_STATETYPE state)
/* No need to check if anything has changed here */
done:
gst_omx_rec_mutex_unlock (&comp->state_lock);
gst_omx_rec_mutex_unlock_for_recursion (&comp->state_lock);
if (err != OMX_ErrorNone) {
GST_ERROR_OBJECT (comp->parent,
@ -1076,7 +1076,7 @@ gst_omx_port_release_buffer (GstOMXPort * port, GstOMXBuffer * buf)
GST_DEBUG_OBJECT (comp->parent, "Releasing buffer %p (%p) to port %u",
buf, buf->omx_buf->pBuffer, port->index);
gst_omx_rec_mutex_lock (&port->port_lock);
gst_omx_rec_mutex_lock_for_recursion (&port->port_lock);
if (port->port_def.eDir == OMX_DirInput) {
/* Reset all flags, some implementations don't
@ -1120,7 +1120,7 @@ gst_omx_port_release_buffer (GstOMXPort * port, GstOMXBuffer * buf)
buf, port->index, gst_omx_error_to_string (err), err);
done:
gst_omx_rec_mutex_unlock (&port->port_lock);
gst_omx_rec_mutex_unlock_for_recursion (&port->port_lock);
return err;
}
@ -1137,7 +1137,7 @@ gst_omx_port_set_flushing (GstOMXPort * port, gboolean flush)
GST_DEBUG_OBJECT (comp->parent, "Setting port %d to %sflushing",
port->index, (flush ? "" : "not "));
gst_omx_rec_mutex_lock (&port->port_lock);
gst_omx_rec_mutex_lock_for_recursion (&port->port_lock);
if (! !flush == ! !port->flushing) {
GST_DEBUG_OBJECT (comp->parent, "Port %u was %sflushing already",
port->index, (flush ? "" : "not "));
@ -1275,7 +1275,7 @@ gst_omx_port_set_flushing (GstOMXPort * port, gboolean flush)
done:
GST_DEBUG_OBJECT (comp->parent, "Set port %u to %sflushing: %s (0x%08x)",
port->index, (flush ? "" : "not "), gst_omx_error_to_string (err), err);
gst_omx_rec_mutex_unlock (&port->port_lock);
gst_omx_rec_mutex_unlock_for_recursion (&port->port_lock);
return err;
@ -1421,9 +1421,9 @@ gst_omx_port_allocate_buffers (GstOMXPort * port)
g_return_val_if_fail (port != NULL, OMX_ErrorUndefined);
gst_omx_rec_mutex_lock (&port->port_lock);
gst_omx_rec_mutex_lock_for_recursion (&port->port_lock);
err = gst_omx_port_allocate_buffers_unlocked (port);
gst_omx_rec_mutex_unlock (&port->port_lock);
gst_omx_rec_mutex_unlock_for_recursion (&port->port_lock);
return err;
}
@ -1515,9 +1515,9 @@ gst_omx_port_deallocate_buffers (GstOMXPort * port)
g_return_val_if_fail (port != NULL, OMX_ErrorUndefined);
gst_omx_rec_mutex_lock (&port->port_lock);
gst_omx_rec_mutex_lock_for_recursion (&port->port_lock);
err = gst_omx_port_deallocate_buffers_unlocked (port);
gst_omx_rec_mutex_unlock (&port->port_lock);
gst_omx_rec_mutex_unlock_for_recursion (&port->port_lock);
return err;
}
@ -1721,9 +1721,9 @@ gst_omx_port_set_enabled (GstOMXPort * port, gboolean enabled)
g_return_val_if_fail (port != NULL, OMX_ErrorUndefined);
gst_omx_rec_mutex_lock (&port->port_lock);
gst_omx_rec_mutex_lock_for_recursion (&port->port_lock);
err = gst_omx_port_set_enabled_unlocked (port, enabled);
gst_omx_rec_mutex_unlock (&port->port_lock);
gst_omx_rec_mutex_unlock_for_recursion (&port->port_lock);
return err;
}
@ -1762,7 +1762,7 @@ gst_omx_port_reconfigure (GstOMXPort * port)
GST_DEBUG_OBJECT (comp->parent, "Reconfiguring port %u", port->index);
gst_omx_rec_mutex_lock (&port->port_lock);
gst_omx_rec_mutex_lock_for_recursion (&port->port_lock);
if (!port->settings_changed)
goto done;
@ -1809,7 +1809,8 @@ done:
GST_DEBUG_OBJECT (comp->parent, "Reconfigured port %u: %s (0x%08x)",
port->index, gst_omx_error_to_string (err), err);
gst_omx_rec_mutex_unlock (&port->port_lock);
gst_omx_rec_mutex_unlock_for_recursion (&port->port_lock);
return err;
}

42
omx/gstomxrecmutex.c
View file

@ -51,6 +51,31 @@ gst_omx_rec_mutex_unlock (GstOMXRecMutex * mutex)
g_mutex_unlock (&mutex->lock);
}
void
gst_omx_rec_mutex_lock_for_recursion (GstOMXRecMutex * mutex)
{
gboolean exchanged;
g_mutex_lock (&mutex->lock);
exchanged =
g_atomic_int_compare_and_exchange (&mutex->recursion_pending, FALSE,
TRUE);
g_assert (exchanged);
}
void
gst_omx_rec_mutex_unlock_for_recursion (GstOMXRecMutex * mutex)
{
gboolean exchanged;
exchanged =
g_atomic_int_compare_and_exchange (&mutex->recursion_pending, TRUE,
FALSE);
g_assert (exchanged);
g_cond_broadcast (&mutex->recursion_wait_cond);
g_mutex_unlock (&mutex->lock);
}
/* must be called with mutex->lock taken */
void
gst_omx_rec_mutex_begin_recursion (GstOMXRecMutex * mutex)
@ -62,6 +87,11 @@ gst_omx_rec_mutex_begin_recursion (GstOMXRecMutex * mutex)
g_atomic_int_compare_and_exchange (&mutex->recursion_allowed, FALSE,
TRUE);
g_assert (exchanged);
exchanged =
g_atomic_int_compare_and_exchange (&mutex->recursion_pending, TRUE,
FALSE);
g_assert (exchanged);
g_cond_broadcast (&mutex->recursion_wait_cond);
g_mutex_unlock (&mutex->recursion_lock);
}
@ -76,6 +106,10 @@ gst_omx_rec_mutex_end_recursion (GstOMXRecMutex * mutex)
g_atomic_int_compare_and_exchange (&mutex->recursion_allowed, TRUE,
FALSE);
g_assert (exchanged);
exchanged =
g_atomic_int_compare_and_exchange (&mutex->recursion_pending, FALSE,
TRUE);
g_assert (exchanged);
g_mutex_unlock (&mutex->recursion_lock);
}
@ -83,10 +117,16 @@ void
gst_omx_rec_mutex_recursive_lock (GstOMXRecMutex * mutex)
{
g_mutex_lock (&mutex->recursion_lock);
if (!g_atomic_int_get (&mutex->recursion_allowed)) {
/* If recursion is pending wait until it happened */
while (g_atomic_int_get (&mutex->recursion_pending))
g_cond_wait (&mutex->recursion_wait_cond, &mutex->recursion_lock);
if (!g_atomic_int_get (&mutex->recursion_allowed)) {
/* no recursion allowed, lock the proper mutex */
g_mutex_unlock (&mutex->recursion_lock);
g_mutex_lock (&mutex->lock);
g_mutex_unlock (&mutex->recursion_lock);
}
}
}

9
omx/gstomxrecmutex.h
View file

@ -92,6 +92,12 @@ struct _GstOMXRecMutex {
* This variable is protected by both locks.
*/
volatile gint recursion_allowed;
/* Indicates whether lock is locked and recursion
* will be allowed soon
*/
volatile gint recursion_pending;
GCond recursion_wait_cond;
};
void gst_omx_rec_mutex_init (GstOMXRecMutex * mutex);
@ -100,6 +106,9 @@ void gst_omx_rec_mutex_clear (GstOMXRecMutex * mutex);
void gst_omx_rec_mutex_lock (GstOMXRecMutex * mutex);
void gst_omx_rec_mutex_unlock (GstOMXRecMutex * mutex);
void gst_omx_rec_mutex_lock_for_recursion (GstOMXRecMutex * mutex);
void gst_omx_rec_mutex_unlock_for_recursion (GstOMXRecMutex * mutex);
void gst_omx_rec_mutex_begin_recursion (GstOMXRecMutex * mutex);
void gst_omx_rec_mutex_end_recursion (GstOMXRecMutex * mutex);