From e90e041e52410ad760a57ed0f1ce15d5a89018db Mon Sep 17 00:00:00 2001 From: Thiago Santos Date: Tue, 11 Oct 2011 23:13:00 -0300 Subject: [PATCH] camerabin2: Add a mutex to avoid concurrent access of preview filename lists The preview filename list is acessed whenever a new capture is started, when camera-source posts a new preview message or on state changes. All of those can occur simultaneously, so add a mutex to prevent concurrent access. --- gst/camerabin2/gstcamerabin2.c | 8 ++++++++ gst/camerabin2/gstcamerabin2.h | 16 +++++++++++++++- 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/gst/camerabin2/gstcamerabin2.c b/gst/camerabin2/gstcamerabin2.c index a91f109c76..fcd98be2ce 100644 --- a/gst/camerabin2/gstcamerabin2.c +++ b/gst/camerabin2/gstcamerabin2.c @@ -406,8 +406,10 @@ gst_camera_bin_start_capture (GstCameraBin2 * camerabin) } /* store the next preview filename */ + g_mutex_lock (camerabin->preview_list_mutex); camerabin->preview_location_list = g_slist_append (camerabin->preview_location_list, location); + g_mutex_unlock (camerabin->preview_list_mutex); g_signal_emit_by_name (camerabin->src, "start-capture", NULL); if (camerabin->mode == MODE_VIDEO && camerabin->audio_src) @@ -518,6 +520,7 @@ gst_camera_bin_dispose (GObject * object) GstCameraBin2 *camerabin = GST_CAMERA_BIN2_CAST (object); g_free (camerabin->location); + g_mutex_free (camerabin->preview_list_mutex); if (camerabin->src_capture_notify_id) g_signal_handler_disconnect (camerabin->src, @@ -873,6 +876,7 @@ gst_camera_bin_init (GstCameraBin2 * camera) camera->zoom = DEFAULT_ZOOM; camera->max_zoom = MAX_ZOOM; camera->flags = DEFAULT_FLAGS; + camera->preview_list_mutex = g_mutex_new (); /* capsfilters are created here as we proxy their caps properties and * this way we avoid having to store the caps while on NULL state to @@ -947,12 +951,14 @@ gst_camera_bin_handle_message (GstBin * bin, GstMessage * message) GValue *value; gchar *location; + g_mutex_lock (camerabin->preview_list_mutex); location = camerabin->preview_location_list->data; camerabin->preview_location_list = g_slist_delete_link (camerabin->preview_location_list, camerabin->preview_location_list); GST_DEBUG_OBJECT (camerabin, "Adding preview location to preview " "message '%s'", location); + g_mutex_unlock (camerabin->preview_list_mutex); value = g_new0 (GValue, 1); g_value_init (value, G_TYPE_STRING); @@ -1725,9 +1731,11 @@ gst_camera_bin_change_state (GstElement * element, GstStateChange trans) g_slist_free (camera->image_location_list); camera->image_location_list = NULL; + g_mutex_lock (camera->preview_list_mutex); g_slist_foreach (camera->preview_location_list, (GFunc) g_free, NULL); g_slist_free (camera->preview_location_list); camera->preview_location_list = NULL; + g_mutex_unlock (camera->preview_list_mutex); /* explicitly set to READY as they might be outside of the bin */ gst_element_set_state (camera->audio_volume, GST_STATE_READY); diff --git a/gst/camerabin2/gstcamerabin2.h b/gst/camerabin2/gstcamerabin2.h index 05af34f4de..3b4c9c92fd 100644 --- a/gst/camerabin2/gstcamerabin2.h +++ b/gst/camerabin2/gstcamerabin2.h @@ -94,8 +94,22 @@ struct _GstCameraBin2 * each buffer capture */ GSList *image_location_list; - /* similar to above, but used for giving names to previews */ + /* + * Similar to above, but used for giving names to previews + * + * Need to protect with a mutex as this list is used when the + * camera-source posts a preview image. As we have no control + * on how the camera-source will behave (we can only tell how + * it should), the preview location list might be used in an + * inconsistent way. + * One example is the camera-source posting a preview image after + * camerabin2 was put to ready, when this preview list will be + * freed and set to NULL. Concurrent access might lead to crashes in + * this situation. (Concurrency from the state-change freeing the + * list and the message handling function looking at preview names) + */ GSList *preview_location_list; + GMutex *preview_list_mutex; gboolean video_profile_switch; gboolean image_profile_switch;