After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 763762 - crash due to gst_iterator_fold which lacks error handling.
crash due to gst_iterator_fold which lacks error handling.
Product: GStreamer
Classification: Platform
Component: gstreamer (core)
git master
Other Linux
: Normal normal
: 1.9.1
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Reported: 2016-03-16 15:35 UTC by Kit Park
Modified: 2016-05-23 17:11 UTC
See Also:
GNOME target: ---
GNOME version: ---

Description Kit Park 2016-03-16 15:35:18 UTC

I have seen a crash when do pipeline destruction which caused by our own problem. However, as far as I see, this reveals the lack of handling in GST since gst_iterator_fold() unset the uninitialized value in case of errors and which is wrong. Hence a crash.

gst_iterator_fold (GstIterator * it, GstIteratorFoldFunction func,
    GValue * ret, gpointer user_data)
  GValue item = { 0, };
  GstIteratorResult result;

  while (1) {
      // ...
        goto fold_done;
        goto fold_done;

  g_value_unset (&item);

  return result;

  • #0 g_value_unset
    from /lib/
  • #1 gst_iterator_fold
    at gstiterator.c line 628
  • #2 bin_iterate_fold
    at gstbin.c line 3997
  • #3 gst_bin_query
    at gstbin.c line 4115

Hope this is clear to fix that.
Many thanks
Comment 1 Jan Schmidt 2016-03-16 15:42:43 UTC
Yes, it should not unset if gst_iterator_next returned GST_ITERATOR_ERROR without initialising the g_value - which has happened in this case because the iterator is NULL for some reason.

How did it end up in that situation?
Comment 2 Kit Park 2016-03-16 15:48:39 UTC
With some luck! That happened when tries to access a pipeline which has lots of elements in it while changing state to NULL.
Comment 3 Jan Schmidt 2016-03-16 15:51:44 UTC
AFAICS, any path that leads to that situation would first throw a CRITICAL that the bin is invalid, which implies a ref-counting error somewhere.
Comment 4 Kit Park 2016-03-16 15:55:17 UTC
Thanks. So you mean that ref-counting error somewhere in user code which is our code in dtor in this case?
Comment 5 Jan Schmidt 2016-03-16 16:25:33 UTC
An extra unref somewhere in your code is the most likely. It's not impossible that it could be inside GStreamer somewhere, but it's more likely we'd have already seen that if so.
Comment 6 Kit Park 2016-03-16 17:16:48 UTC
Thanks for a comment and will look into our side. BTW, this bug should be fixed or is it rather good to let the user crash?
Comment 7 Jan Schmidt 2016-03-16 17:33:40 UTC
I'm not sure I understand the crash still, actually. Did you capture any output from the app when it crashed? Is it hard to get it to happen again?
Comment 8 Kit Park 2016-03-17 11:25:00 UTC
What output did you mean? GST log? Not that hard to reproduce since narrowed down how to make it happen on purpose. 

The question is: the app might get wrong about keeping ref-count to GST elements and which may cause to take this path leading to this. Need to look into the app side further. But separately, wondered whether this NULL iterator handling in this function is rather well-intended. May be okay to let the app crash but the app is not calling that explicitly with null iterator in this case.

Hope this is clearer.
Comment 9 Jan Schmidt 2016-03-17 11:35:38 UTC
Yes, I meant any stderr output preceding the fault. It also should not crash in g_value_unset unless you have G_DEBUG=fatal-warnings on. The GValue doesn't hold a value, but it's not uninitialised - it's been initialised to 0 and should just warn on g_value_unset.... so if it's actually crashing there's something here I don't understand yet.
Comment 10 Kit Park 2016-03-22 18:09:28 UTC
No log on stderr and no G_DEBUG setting. I assume that you're taking about `g_return_if_fail()` in the code?

g_value_unset (GValue *value)
  GTypeValueTable *value_table;
  g_return_if_fail (G_IS_VALUE (value));

  value_table = g_type_value_table_peek (G_VALUE_TYPE (value));

  if (value_table->value_free)
    value_table->value_free (value);
  memset (value, 0, sizeof (*value));
Comment 11 Tim-Philipp Müller 2016-05-23 17:11:54 UTC
We can't assume that the g_return_if_fail() checks are active, GLib might have built with -DG_DISABLE_CHECKS or whatever the define is.

Pushed this now:

 commit e0de5ed915f779eaefac7267a54f3aa49837a0bc
 Author: Tim-Philipp Müller <>
 Date:   Mon May 23 18:00:30 2016 +0100

    iterator: only unset GValue if it was inited
    And add some function guards. From GLib 2.48 on it is
    allowed to pass an uninitialised GValue to g_value_unset().

But really, if you call _fold() with a NULL iterator, all bets are off already, things went wrong before already then.