Created attachment 312344 [details] [review]
[1/2] fix argument order

Created attachment 312345 [details] [review]
[2/2] free GClosures

Review of attachment 312344 [details] [review]:

That sounds ridiculusly unconsistent!

Review of attachment 312345 [details] [review]:

::: plugins/nle/nlecomposition.c
@@ +335,2 @@
       g_closure_unref ((GClosure *) act);
+      comp->priv->actions = g_list_remove_link (comp->priv->actions, removed);

I do not understand why you can not use g_list_delete_link as it should free the node as we want, what am I missing? :)

@@ +378,3 @@
     g_value_set_object (&params[0], comp);
 
+    lact = g_list_first (priv->actions);

This is the same thing.

@@ +1019,3 @@
   priv->dispose_has_run = TRUE;
 
+  g_list_foreach (priv->objects_start, _remove_each_nleobj, comp);

g_list_free full?

(some for the followings :))

@@ +2342,3 @@
+  }
+
+  ret = GST_ELEMENT_CLASS (parent_class)->change_state (element, transition);

Why did you move that?

I do not think it is related to the leaks is it?

Review of attachment 312345 [details] [review]:

::: plugins/nle/nlecomposition.c
@@ +335,2 @@
       g_closure_unref ((GClosure *) act);
+      comp->priv->actions = g_list_remove_link (comp->priv->actions, removed);

In the for loop, tmp should be valid for next turn, but, I thought, 'tmp' variable could be invalid after calling g_list_delete_link.

The previous code might be same as below.

for (....; tmp->next) {
  g_list_remove_link (actions, tmp);
  g_list_free (tmp); 
}

In this code, for loop will access 'tmp' variable after freed.

So I introduced 'removed' variable to free orphan item safely.

@@ +1019,3 @@
   priv->dispose_has_run = TRUE;
 
+  g_list_foreach (priv->objects_start, _remove_each_nleobj, comp);

Once I tried to use "g_list_free_full".
But I cannot get NleComposition pointer properly because g_list_free_full doesn't have user_data argument.

@@ +2342,3 @@
+  }
+
+  ret = GST_ELEMENT_CLASS (parent_class)->change_state (element, transition);

I tend to give more chance to invoke gclosures when the element goes NULL state.
Even after setting NULL state to nlecomposition element, Actions are consistently added to the list of Action(priv->actions).

The term until parent element finish its state transition could be very small, but, 
I thought, it would be better to invoke them rather than just to drop. If task stops, each action will not be invoked and should be dropped in dispose function.

Review of attachment 312345 [details] [review]:

::: plugins/nle/nlecomposition.c
@@ +335,2 @@
       g_closure_unref ((GClosure *) act);
+      comp->priv->actions = g_list_remove_link (comp->priv->actions, removed);

OK

@@ +1019,3 @@
   priv->dispose_has_run = TRUE;
 
+  g_list_foreach (priv->objects_start, _remove_each_nleobj, comp);

Indeed, all right.

@@ +2342,3 @@
+  }
+
+  ret = GST_ELEMENT_CLASS (parent_class)->change_state (element, transition);

I still do not understand what exact problem it solve.

Review of attachment 312345 [details] [review]:

::: plugins/nle/nlecomposition.c
@@ +2342,3 @@
+  }
+
+  ret = GST_ELEMENT_CLASS (parent_class)->change_state (element, transition);

okay, to be honest, I also not sure 100% :)
I'll remove this block and attach again.

Created attachment 312459 [details] [review]
[2/2] free GClosures

Created attachment 312460 [details] [review]
[2/2] free GClosures

Created attachment 312461 [details] [review]
[2/2] free GClosures

Review of attachment 312461 [details] [review]:

This patch causes timeout randomly with gst-validate-launcher testing.
I might find another solution.

Review of attachment 312461 [details] [review]:

sorry for false reporting. this is not related to randomly timeout issue which I've reported above.
please, review this code again.
thank you.

These are moved in phabricator.

https://phabricator.freedesktop.org/D324
https://phabricator.freedesktop.org/D323