gst/gstbin.c: Keep track of pads that are being linked/unlinked and resync the state changes.

Original commit message from CVS:
Base on Patch by: Olivier Crete <tester at tester dot ca>
* gst/gstbin.c: (gst_bin_init), (gst_bin_add_func),
(gst_bin_remove_func), (update_degree),
(gst_bin_sort_iterator_new), (gst_bin_handle_message_func):
Keep track of pads that are being linked/unlinked and resync the state
changes.
* gst/gstpad.c: (gst_pad_get_direction),
(gst_pad_set_chain_function), (gst_pad_set_getrange_function),
(gst_pad_set_checkgetrange_function), (gst_pad_unlink),
(gst_pad_link_prepare), (gst_pad_link),
(gst_pad_event_default_dispatch), (gst_pad_chain), (gst_pad_push),
(gst_pad_check_pull_range), (gst_pad_get_range),
(gst_pad_pull_range):
Some code cleanups, use macros to check pad direction.
Don't need to take the lock on the pad direction.
Post structure change when pads are linked/unlinked.
Change some checks into _return_if_fail().
* tests/check/gst/gstbin.c:
(test_link_structure_change_state_changed_sync_cb),
(GST_START_TEST), (gst_bin_suite):
Add testcase for pad link/unlinke resync during a state change.
Fixes #510354.

ChangeLog

@@ -1,3 +1,31 @@
+2008-10-06  Wim Taymans  <wim.taymans@collabora.co.uk>
+
+	Base on Patch by: Olivier Crete <tester at tester dot ca>
+
+	* gst/gstbin.c: (gst_bin_init), (gst_bin_add_func),
+	(gst_bin_remove_func), (update_degree),
+	(gst_bin_sort_iterator_new), (gst_bin_handle_message_func):
+	Keep track of pads that are being linked/unlinked and resync the state
+	changes.
+
+	* gst/gstpad.c: (gst_pad_get_direction),
+	(gst_pad_set_chain_function), (gst_pad_set_getrange_function),
+	(gst_pad_set_checkgetrange_function), (gst_pad_unlink),
+	(gst_pad_link_prepare), (gst_pad_link),
+	(gst_pad_event_default_dispatch), (gst_pad_chain), (gst_pad_push),
+	(gst_pad_check_pull_range), (gst_pad_get_range),
+	(gst_pad_pull_range):
+	Some code cleanups, use macros to check pad direction.
+	Don't need to take the lock on the pad direction.
+	Post structure change when pads are linked/unlinked.
+	Change some checks into _return_if_fail().
+
+	* tests/check/gst/gstbin.c:
+	(test_link_structure_change_state_changed_sync_cb),
+	(GST_START_TEST), (gst_bin_suite):
+	Add testcase for pad link/unlinke resync during a state change.
+	Fixes #510354.
+
 2008-10-06  Wim Taymans  <wim.taymans@collabora.co.uk>
 
 	* docs/gst/gstreamer-sections.txt:

gst/gstbin.c

@@ -201,6 +201,8 @@ struct _GstBinPrivate
    * have to process the message after finishing the state change even when no
    * child returned GST_STATE_CHANGE_ASYNC. */
   gboolean pending_async_done;
+
+  guint32 structure_cookie;
 };
 
 typedef struct
@@ -485,6 +487,7 @@ gst_bin_init (GstBin * bin)
 
   bin->priv = GST_BIN_GET_PRIVATE (bin);
   bin->priv->asynchandling = DEFAULT_ASYNC_HANDLING;
+  bin->priv->structure_cookie = 0;
 }
 
 static void
@@ -748,7 +751,7 @@ find_message (GstBin * bin, GstObject * src, GstMessageType types)
   return result;
 }
 
-/* with LOCK, returns TRUE if message had a valid SRC, takes ref on
+/* with LOCK, returns TRUE if message had a valid SRC, takes ownership of
  * the message.
  *
  * A message that is cached and has the same SRC and type is replaced
@@ -916,6 +919,7 @@ gst_bin_add_func (GstBin * bin, GstElement * element)
   bin->children = g_list_prepend (bin->children, element);
   bin->numchildren++;
   bin->children_cookie++;
+  bin->priv->structure_cookie++;
 
   /* distribute the bus */
   gst_element_set_bus (element, bin->child_bus);
@@ -1125,6 +1129,7 @@ gst_bin_remove_func (GstBin * bin, GstElement * element)
    * so that others can detect a change in the children list. */
   bin->numchildren--;
   bin->children_cookie++;
+  bin->priv->structure_cookie++;
 
   if (is_sink && !othersink) {
     /* we're not a sink anymore */
@@ -1150,19 +1155,44 @@ gst_bin_remove_func (GstBin * bin, GstElement * element)
   for (walk = bin->messages; walk; walk = next) {
     GstMessage *message = (GstMessage *) walk->data;
     GstElement *src = GST_ELEMENT_CAST (GST_MESSAGE_SRC (message));
+    gboolean remove;
 
     next = g_list_next (walk);
+    remove = FALSE;
 
-    if (GST_MESSAGE_TYPE (message) == GST_MESSAGE_ASYNC_START) {
+    switch (GST_MESSAGE_TYPE (message)) {
-      if (src == element)
+      case GST_MESSAGE_ASYNC_START:
-        this_async = TRUE;
+        if (src == element)
-      else
+          this_async = TRUE;
-        other_async = TRUE;
+        else
+          other_async = TRUE;
 
-      GST_DEBUG_OBJECT (GST_MESSAGE_SRC (message),
+        GST_DEBUG_OBJECT (GST_MESSAGE_SRC (message),
-          "looking at message %p", message);
+            "looking at message %p", message);
+        break;
+      case GST_MESSAGE_STRUCTURE_CHANGE:
+      {
+        GstElement *owner;
+
+        GST_DEBUG_OBJECT (GST_MESSAGE_SRC (message),
+            "looking at structure change message %p", message);
+        /* it's unlikely that this message is still in the list of messages
+         * because this would mean that a link/unlink is busy in another thread
+         * while we remove the element. We still have to remove the message
+         * because we might not receive the done message anymore when the element
+         * is removed from the bin. */
+        gst_message_parse_structure_change (message, NULL, &owner, NULL);
+        if (owner == element)
+          remove = TRUE;
+        break;
+      }
+      default:
+        break;
     }
-    if (src == element) {
+    if (src == element)
+      remove = TRUE;
+
+    if (remove) {
       /* delete all message types */
       GST_DEBUG_OBJECT (GST_MESSAGE_SRC (message),
           "deleting message %p of element \"%s\"", message, elem_name);
@@ -1652,18 +1682,28 @@ update_degree (GstElement * element, GstBinSortIterator * bit)
   gboolean linked = FALSE;
 
   GST_OBJECT_LOCK (element);
-  /* don't touch degree if element has no sourcepads */
+  /* don't touch degree if element has no sinkpads */
   if (element->numsinkpads != 0) {
     /* loop over all sinkpads, decrement degree for all connected
      * elements in this bin */
     GList *pads;
 
     for (pads = element->sinkpads; pads; pads = g_list_next (pads)) {
-      GstPad *peer;
+      GstPad *pad, *peer;
 
-      if ((peer = gst_pad_get_peer (GST_PAD_CAST (pads->data)))) {
+      pad = GST_PAD_CAST (pads->data);
+
+      if ((peer = gst_pad_get_peer (pad))) {
         GstElement *peer_element;
 
+        /* we're iterating over the sinkpads, this is the peer and thus the
+         * srcpad, check if it's busy in a link/unlink */
+        if (G_UNLIKELY (find_message (bit->bin, GST_OBJECT_CAST (peer),
+                    GST_MESSAGE_STRUCTURE_CHANGE))) {
+          gst_object_unref (peer);
+          continue;
+        }
+
         if ((peer_element = gst_pad_get_parent_element (peer))) {
           GST_OBJECT_LOCK (peer_element);
           /* check that we don't go outside of this bin */
@@ -1686,7 +1726,10 @@ update_degree (GstElement * element, GstBinSortIterator * bit)
                 GST_ELEMENT_NAME (peer_element), old_deg, new_deg,
                 GST_ELEMENT_NAME (element));
 
-            /* update degree */
+            /* update degree, it is possible that an element was in 0 and
+             * reaches -1 here. This would mean that the element had no sinkpads
+             * but became linked while the state change was happening. We will
+             * resync on this with the structure change message. */
             if (new_deg == 0) {
               /* degree hit 0, add to queue */
               add_to_queue (bit, peer_element);
@@ -1814,7 +1857,7 @@ gst_bin_sort_iterator_new (GstBin * bin)
       gst_iterator_new (sizeof (GstBinSortIterator),
       GST_TYPE_ELEMENT,
       GST_OBJECT_GET_LOCK (bin),
-      &bin->children_cookie,
+      &bin->priv->structure_cookie,
       (GstIteratorNextFunction) gst_bin_sort_iterator_next,
       (GstIteratorItemFunction) NULL,
       (GstIteratorResyncFunction) gst_bin_sort_iterator_resync,
@@ -2928,6 +2971,34 @@ gst_bin_handle_message_func (GstBin * bin, GstMessage * message)
         break;
       }
     }
+    case GST_MESSAGE_STRUCTURE_CHANGE:
+    {
+      gboolean busy;
+
+      gst_message_parse_structure_change (message, NULL, NULL, &busy);
+
+      GST_OBJECT_LOCK (bin);
+      if (busy) {
+        /* while the pad is busy, avoid following it when doing state changes.
+         * Don't update the cookie yet, we will do that after the structure
+         * change finished and we are ready to inspect the new updated
+         * structure. */
+        bin_replace_message (bin, message, GST_MESSAGE_STRUCTURE_CHANGE);
+        message = NULL;
+      } else {
+        /* a pad link/unlink ended, signal the state change iterator that we
+         * need to resync by updating the structure_cookie. */
+        bin_remove_messages (bin, GST_MESSAGE_SRC (message),
+            GST_MESSAGE_STRUCTURE_CHANGE);
+        bin->priv->structure_cookie++;
+      }
+      GST_OBJECT_UNLOCK (bin);
+
+      if (message)
+        gst_message_unref (message);
+
+      break;
+    }
     default:
       goto forward;
   }

gst/gstpad.c

@@ -559,9 +559,7 @@ gst_pad_get_direction (GstPad * pad)
    * error return value */
   g_return_val_if_fail (GST_IS_PAD (pad), GST_PAD_UNKNOWN);
 
-  GST_OBJECT_LOCK (pad);
   result = GST_PAD_DIRECTION (pad);
-  GST_OBJECT_UNLOCK (pad);
 
   return result;
 }
@@ -1185,7 +1183,7 @@ void
 gst_pad_set_chain_function (GstPad * pad, GstPadChainFunction chain)
 {
   g_return_if_fail (GST_IS_PAD (pad));
-  g_return_if_fail (GST_PAD_DIRECTION (pad) == GST_PAD_SINK);
+  g_return_if_fail (GST_PAD_IS_SINK (pad));
 
   GST_PAD_CHAINFUNC (pad) = chain;
   GST_CAT_DEBUG_OBJECT (GST_CAT_PADS, pad, "chainfunc set to %s",
@@ -1205,7 +1203,7 @@ void
 gst_pad_set_getrange_function (GstPad * pad, GstPadGetRangeFunction get)
 {
   g_return_if_fail (GST_IS_PAD (pad));
-  g_return_if_fail (GST_PAD_DIRECTION (pad) == GST_PAD_SRC);
+  g_return_if_fail (GST_PAD_IS_SRC (pad));
 
   GST_PAD_GETRANGEFUNC (pad) = get;
 
@@ -1226,7 +1224,7 @@ gst_pad_set_checkgetrange_function (GstPad * pad,
     GstPadCheckGetRangeFunction check)
 {
   g_return_if_fail (GST_IS_PAD (pad));
-  g_return_if_fail (GST_PAD_DIRECTION (pad) == GST_PAD_SRC);
+  g_return_if_fail (GST_PAD_IS_SRC (pad));
 
   GST_PAD_CHECKGETRANGEFUNC (pad) = check;
 
@@ -1567,23 +1565,36 @@ gst_pad_set_bufferalloc_function (GstPad * pad,
 gboolean
 gst_pad_unlink (GstPad * srcpad, GstPad * sinkpad)
 {
+  gboolean result = FALSE;
+  GstElement *parent = NULL;
+
   g_return_val_if_fail (GST_IS_PAD (srcpad), FALSE);
+  g_return_val_if_fail (GST_PAD_IS_SRC (srcpad), FALSE);
   g_return_val_if_fail (GST_IS_PAD (sinkpad), FALSE);
+  g_return_val_if_fail (GST_PAD_IS_SINK (sinkpad), FALSE);
 
   GST_CAT_INFO (GST_CAT_ELEMENT_PADS, "unlinking %s:%s(%p) and %s:%s(%p)",
       GST_DEBUG_PAD_NAME (srcpad), srcpad,
       GST_DEBUG_PAD_NAME (sinkpad), sinkpad);
 
+  /* We need to notify the parent before taking any pad locks as the bin in
+   * question might be waiting for a lock on the pad while holding its lock
+   * that our message will try to take. */
+  if ((parent = GST_ELEMENT_CAST (gst_pad_get_parent (srcpad)))) {
+    if (GST_IS_ELEMENT (parent)) {
+      gst_element_post_message (parent,
+          gst_message_new_structure_change (GST_OBJECT_CAST (srcpad),
+              GST_STRUCTURE_CHANGE_TYPE_PAD_UNLINK, parent, TRUE));
+    } else {
+      gst_object_unref (parent);
+      parent = NULL;
+    }
+  }
+
   GST_OBJECT_LOCK (srcpad);
 
-  if (G_UNLIKELY (GST_PAD_DIRECTION (srcpad) != GST_PAD_SRC))
-    goto not_srcpad;
-
   GST_OBJECT_LOCK (sinkpad);
 
-  if (G_UNLIKELY (GST_PAD_DIRECTION (sinkpad) != GST_PAD_SINK))
-    goto not_sinkpad;
-
   if (G_UNLIKELY (GST_PAD_PEER (srcpad) != sinkpad))
     goto not_linked_together;
 
@@ -1609,28 +1620,25 @@ gst_pad_unlink (GstPad * srcpad, GstPad * sinkpad)
   GST_CAT_INFO (GST_CAT_ELEMENT_PADS, "unlinked %s:%s and %s:%s",
       GST_DEBUG_PAD_NAME (srcpad), GST_DEBUG_PAD_NAME (sinkpad));
 
-  return TRUE;
+  result = TRUE;
 
-not_srcpad:
+done:
-  {
+  if (parent != NULL) {
-    g_critical ("pad %s is not a source pad", GST_PAD_NAME (srcpad));
+    gst_element_post_message (parent,
-    GST_OBJECT_UNLOCK (srcpad);
+        gst_message_new_structure_change (GST_OBJECT_CAST (srcpad),
-    return FALSE;
+            GST_STRUCTURE_CHANGE_TYPE_PAD_UNLINK, parent, FALSE));
-  }
+    gst_object_unref (parent);
-not_sinkpad:
-  {
-    g_critical ("pad %s is not a sink pad", GST_PAD_NAME (sinkpad));
-    GST_OBJECT_UNLOCK (sinkpad);
-    GST_OBJECT_UNLOCK (srcpad);
-    return FALSE;
   }
+  return result;
+
+  /* ERRORS */
 not_linked_together:
   {
     /* we do not emit a warning in this case because unlinking cannot
      * be made MT safe.*/
     GST_OBJECT_UNLOCK (sinkpad);
     GST_OBJECT_UNLOCK (srcpad);
-    return FALSE;
+    goto done;
   }
 }
 
@@ -1793,26 +1801,16 @@ wrong_grandparents:
 static GstPadLinkReturn
 gst_pad_link_prepare (GstPad * srcpad, GstPad * sinkpad)
 {
-  /* generic checks */
-  g_return_val_if_fail (GST_IS_PAD (srcpad), GST_PAD_LINK_REFUSED);
-  g_return_val_if_fail (GST_IS_PAD (sinkpad), GST_PAD_LINK_REFUSED);
-
   GST_CAT_INFO (GST_CAT_PADS, "trying to link %s:%s and %s:%s",
       GST_DEBUG_PAD_NAME (srcpad), GST_DEBUG_PAD_NAME (sinkpad));
 
   GST_OBJECT_LOCK (srcpad);
 
-  if (G_UNLIKELY (GST_PAD_DIRECTION (srcpad) != GST_PAD_SRC))
-    goto not_srcpad;
-
   if (G_UNLIKELY (GST_PAD_PEER (srcpad) != NULL))
     goto src_was_linked;
 
   GST_OBJECT_LOCK (sinkpad);
 
-  if (G_UNLIKELY (GST_PAD_DIRECTION (sinkpad) != GST_PAD_SINK))
-    goto not_sinkpad;
-
   if (G_UNLIKELY (GST_PAD_PEER (sinkpad) != NULL))
     goto sink_was_linked;
 
@@ -1829,12 +1827,6 @@ gst_pad_link_prepare (GstPad * srcpad, GstPad * sinkpad)
 
   return GST_PAD_LINK_OK;
 
-not_srcpad:
-  {
-    g_critical ("pad %s is not a source pad", GST_PAD_NAME (srcpad));
-    GST_OBJECT_UNLOCK (srcpad);
-    return GST_PAD_LINK_WRONG_DIRECTION;
-  }
 src_was_linked:
   {
     GST_CAT_INFO (GST_CAT_PADS, "src %s:%s was already linked to %s:%s",
@@ -1845,13 +1837,6 @@ src_was_linked:
     GST_OBJECT_UNLOCK (srcpad);
     return GST_PAD_LINK_WAS_LINKED;
   }
-not_sinkpad:
-  {
-    g_critical ("pad %s is not a sink pad", GST_PAD_NAME (sinkpad));
-    GST_OBJECT_UNLOCK (sinkpad);
-    GST_OBJECT_UNLOCK (srcpad);
-    return GST_PAD_LINK_WRONG_DIRECTION;
-  }
 sink_was_linked:
   {
     GST_CAT_INFO (GST_CAT_PADS, "sink %s:%s was already linked to %s:%s",
@@ -1895,12 +1880,31 @@ GstPadLinkReturn
 gst_pad_link (GstPad * srcpad, GstPad * sinkpad)
 {
   GstPadLinkReturn result;
+  GstElement *parent;
+
+  g_return_val_if_fail (GST_IS_PAD (srcpad), GST_PAD_LINK_REFUSED);
+  g_return_val_if_fail (GST_PAD_IS_SRC (srcpad), GST_PAD_LINK_WRONG_DIRECTION);
+  g_return_val_if_fail (GST_IS_PAD (sinkpad), GST_PAD_LINK_REFUSED);
+  g_return_val_if_fail (GST_PAD_IS_SINK (sinkpad),
+      GST_PAD_LINK_WRONG_DIRECTION);
+
+  /* Notify the parent early. See gst_pad_unlink for details. */
+  if ((parent = GST_ELEMENT_CAST (gst_pad_get_parent (srcpad)))) {
+    if (GST_IS_ELEMENT (parent)) {
+      gst_element_post_message (parent,
+          gst_message_new_structure_change (GST_OBJECT_CAST (srcpad),
+              GST_STRUCTURE_CHANGE_TYPE_PAD_LINK, parent, TRUE));
+    } else {
+      gst_object_unref (parent);
+      parent = NULL;
+    }
+  }
 
   /* prepare will also lock the two pads */
   result = gst_pad_link_prepare (srcpad, sinkpad);
 
   if (result != GST_PAD_LINK_OK)
-    goto prepare_failed;
+    goto done;
 
   /* must set peers before calling the link function */
   GST_PAD_PEER (srcpad) = sinkpad;
@@ -1946,12 +1950,16 @@ gst_pad_link (GstPad * srcpad, GstPad * sinkpad)
     GST_OBJECT_UNLOCK (sinkpad);
     GST_OBJECT_UNLOCK (srcpad);
   }
-  return result;
 
-prepare_failed:
+done:
-  {
+  if (parent) {
-    return result;
+    gst_element_post_message (parent,
+        gst_message_new_structure_change (GST_OBJECT_CAST (srcpad),
+            GST_STRUCTURE_CHANGE_TYPE_PAD_LINK, parent, FALSE));
+    gst_object_unref (parent);
   }
+
+  return result;
 }
 
 static void
@@ -3242,7 +3250,7 @@ gst_pad_event_default_dispatch (GstPad * pad, GstEvent * event)
           break;
         }
 
-        if (GST_PAD_DIRECTION (eventpad) == GST_PAD_SRC) {
+        if (GST_PAD_IS_SRC (eventpad)) {
           /* for each pad we send to, we should ref the event; it's up
            * to downstream to unref again when handled. */
           GST_LOG_OBJECT (pad, "Reffing and sending event %p (%s) to %s:%s",
@@ -3287,7 +3295,7 @@ no_iter:
    * return TRUE. This is so that when using the default handler on a sink
    * element, we don't fail to push it. */
   if (!pushed_pads)
-    result = (GST_PAD_DIRECTION (pad) == GST_PAD_SINK);
+    result = GST_PAD_IS_SINK (pad);
 
   g_list_free (pushed_pads);
 
@@ -3957,8 +3965,7 @@ GstFlowReturn
 gst_pad_chain (GstPad * pad, GstBuffer * buffer)
 {
   g_return_val_if_fail (GST_IS_PAD (pad), GST_FLOW_ERROR);
-  g_return_val_if_fail (GST_PAD_DIRECTION (pad) == GST_PAD_SINK,
+  g_return_val_if_fail (GST_PAD_IS_SINK (pad), GST_FLOW_ERROR);
-      GST_FLOW_ERROR);
   g_return_val_if_fail (GST_IS_BUFFER (buffer), GST_FLOW_ERROR);
 
   return gst_pad_chain_unchecked (pad, buffer);
@@ -4000,7 +4007,7 @@ gst_pad_push (GstPad * pad, GstBuffer * buffer)
   gboolean caps_changed;
 
   g_return_val_if_fail (GST_IS_PAD (pad), GST_FLOW_ERROR);
-  g_return_val_if_fail (GST_PAD_DIRECTION (pad) == GST_PAD_SRC, GST_FLOW_ERROR);
+  g_return_val_if_fail (GST_PAD_IS_SRC (pad), GST_FLOW_ERROR);
   g_return_val_if_fail (GST_IS_BUFFER (buffer), GST_FLOW_ERROR);
 
   GST_OBJECT_LOCK (pad);
@@ -4111,7 +4118,7 @@ gst_pad_check_pull_range (GstPad * pad)
   g_return_val_if_fail (GST_IS_PAD (pad), FALSE);
 
   GST_OBJECT_LOCK (pad);
-  if (GST_PAD_DIRECTION (pad) != GST_PAD_SINK)
+  if (!GST_PAD_IS_SINK (pad))
     goto wrong_direction;
 
   if (G_UNLIKELY ((peer = GST_PAD_PEER (pad)) == NULL))
@@ -4189,7 +4196,7 @@ gst_pad_get_range (GstPad * pad, guint64 offset, guint size,
   gboolean emit_signal;
 
   g_return_val_if_fail (GST_IS_PAD (pad), GST_FLOW_ERROR);
-  g_return_val_if_fail (GST_PAD_DIRECTION (pad) == GST_PAD_SRC, GST_FLOW_ERROR);
+  g_return_val_if_fail (GST_PAD_IS_SRC (pad), GST_FLOW_ERROR);
   g_return_val_if_fail (buffer != NULL, GST_FLOW_ERROR);
 
   GST_PAD_STREAM_LOCK (pad);
@@ -4323,8 +4330,7 @@ gst_pad_pull_range (GstPad * pad, guint64 offset, guint size,
   gboolean emit_signal;
 
   g_return_val_if_fail (GST_IS_PAD (pad), GST_FLOW_ERROR);
-  g_return_val_if_fail (GST_PAD_DIRECTION (pad) == GST_PAD_SINK,
+  g_return_val_if_fail (GST_PAD_IS_SINK (pad), GST_FLOW_ERROR);
-      GST_FLOW_ERROR);
   g_return_val_if_fail (buffer != NULL, GST_FLOW_ERROR);
 
   GST_OBJECT_LOCK (pad);

tests/check/gst/gstbin.c

@@ -893,6 +893,82 @@ GST_START_TEST (test_iterate_sorted)
 
 GST_END_TEST;
 
+static void
+test_link_structure_change_state_changed_sync_cb (GstBus * bus,
+    GstMessage * message, gpointer data)
+{
+  GstPipeline *pipeline = GST_PIPELINE (data);
+  GstElement *src, *identity, *sink;
+  GstState old, snew, pending;
+
+  sink = gst_bin_get_by_name (GST_BIN (pipeline), "sink");
+  fail_unless (sink != NULL, "Could not get sink");
+
+  gst_message_parse_state_changed (message, &old, &snew, &pending);
+  if (message->src != GST_OBJECT (sink) || snew != GST_STATE_READY) {
+    gst_object_unref (sink);
+    return;
+  }
+
+  src = gst_bin_get_by_name (GST_BIN (pipeline), "src");
+  fail_unless (src != NULL, "Could not get src");
+
+  identity = gst_bin_get_by_name (GST_BIN (pipeline), "identity");
+  fail_unless (identity != NULL, "Could not get identity");
+
+  /* link src to identity, the pipeline should detect the new link and
+   * resync the state change */
+  fail_unless (gst_element_link (src, identity) == TRUE);
+
+  gst_object_unref (src);
+  gst_object_unref (identity);
+  gst_object_unref (sink);
+}
+
+GST_START_TEST (test_link_structure_change)
+{
+  GstElement *src, *identity, *sink, *pipeline;
+  GstBus *bus;
+  GstState state;
+
+  pipeline = gst_pipeline_new (NULL);
+  fail_unless (pipeline != NULL, "Could not create pipeline");
+
+  bus = gst_pipeline_get_bus (GST_PIPELINE (pipeline));
+  fail_unless (bus != NULL, "Could not get bus");
+
+  /* use the sync signal handler to link elements while the pipeline is still
+   * doing the state change */
+  gst_bus_set_sync_handler (bus, gst_bus_sync_signal_handler, pipeline);
+  g_object_connect (bus, "signal::sync-message::state-changed",
+      G_CALLBACK (test_link_structure_change_state_changed_sync_cb), pipeline,
+      NULL);
+
+  src = gst_element_factory_make ("fakesrc", "src");
+  fail_if (src == NULL, "Could not create fakesrc");
+
+  identity = gst_element_factory_make ("identity", "identity");
+  fail_if (identity == NULL, "Could not create identity");
+
+  sink = gst_element_factory_make ("fakesink", "sink");
+  fail_if (sink == NULL, "Could not create fakesink1");
+
+  gst_bin_add_many (GST_BIN (pipeline), src, identity, sink, NULL);
+
+  gst_element_set_state (pipeline, GST_STATE_READY);
+  gst_element_get_state (pipeline, NULL, NULL, GST_CLOCK_TIME_NONE);
+
+  /* the state change will be done on src only if the pipeline correctly resyncs
+   * after that filesrc has been linked to identity */
+  gst_element_get_state (src, &state, NULL, 0);
+  fail_unless_equals_int (state, GST_STATE_READY);
+
+  gst_object_unref (bus);
+  gst_object_unref (pipeline);
+}
+
+GST_END_TEST;
+
 static Suite *
 gst_bin_suite (void)
 {
@@ -913,6 +989,7 @@ gst_bin_suite (void)
   tcase_add_test (tc_chain, test_add_linked);
   tcase_add_test (tc_chain, test_add_self);
   tcase_add_test (tc_chain, test_iterate_sorted);
+  tcase_add_test (tc_chain, test_link_structure_change);
 
   return s;
 }