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.

diff --git a/gst/camerabin2/gstcamerabin2.c b/gst/camerabin2/gstcamerabin2.c
index a91f109c7679be2afde8e9b43aa7e56c034f684a..fcd98be2ce6c6ae410fb080157932cccc63111f2 100644 (file)
--- 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 05af34f4de8f514485b4961e5638735d031d9500..3b4c9c92fd17fc1c31ce45d72be6052e49c6ee19 100644 (file)
--- 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;