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 624504 - [0.11] [bus] Remove gst_bus_poll()
[0.11] [bus] Remove gst_bus_poll()
Status: RESOLVED WONTFIX
Product: GStreamer
Classification: Platform
Component: gstreamer (core)
0.10.30
Other Linux
: Normal enhancement
: 0.11.x
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2010-07-16 03:15 UTC by Dmytro Poplavskiy
Modified: 2012-09-25 00:11 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
bus: Don't use the default main context in gst_bus_poll() (3.87 KB, patch)
2010-07-16 04:48 UTC, Sebastian Dröge (slomo)
rejected Details | Review
deprecate gst_bus_poll() (29.49 KB, patch)
2010-09-05 23:42 UTC, David Schleef
none Details | Review

Description Dmytro Poplavskiy 2010-07-16 03:15:47 UTC
It should not be used since it runs the main loop from non main thread.

Here is the relevant part of backtrace:


  • #13 QApplication::x11ProcessEvent
    at qt/src/gui/kernel/qapplication_x11.cpp line 3414
  • #14 x11EventSourceDispatch
    at qt/src/gui/kernel/qguieventdispatcher_glib.cpp line 146
  • #15 g_main_context_dispatch
    from /usr/lib/libglib-2.0.so.0
  • #16 ??
    from /usr/lib/libglib-2.0.so.0
  • #17 g_main_loop_run
    from /usr/lib/libglib-2.0.so.0
  • #18 gst_bus_poll
    at gstbus.c line 1058
  • #19 gst_camerabin_preview_convert
    at camerabinpreview.c line 209
  • #20 gst_camerabin_send_preview
    at gstcamerabin.c line 1704
  • #21 gst_camerabin_have_img_buffer
    at gstcamerabin.c line 1755

Comment 1 Sebastian Dröge (slomo) 2010-07-16 03:36:35 UTC
Well, gst_bus_poll() simply shouldn't run a main loop on the default context. Patch coming soon
Comment 2 Sebastian Dröge (slomo) 2010-07-16 04:48:16 UTC
Created attachment 166004 [details] [review]
bus: Don't use the default main context in gst_bus_poll()

Otherwise we might call run a nested main loop in the default
main context, which will confuse GTK+ and other users of the
default main context.

Fixes bug #624504.
Comment 3 Stefan Sauer (gstreamer, gtkdoc dev) 2010-07-16 07:48:16 UTC
The patch looks good, but there is also Bug #615655 for camerabin (shouldn't rely on running GLib main loop).
Comment 4 Sebastian Dröge (slomo) 2010-07-16 08:51:10 UTC
Yes, that's a separate problem though... and very easy to fix.
Comment 5 Tim-Philipp Müller 2010-07-16 09:13:32 UTC
I don't think we can change this now.

This behaviour was at the time regarded as a feature, not a bug, and the problems with it are well-documented in the gst_bus_poll() API documentation.

Use gst_bus_timed_pop_filtered() if you want blocking behaviour without iterating the default main loop.
Comment 6 Sebastian Dröge (slomo) 2010-07-16 09:47:19 UTC
Right, applications might assume that gst_bus_poll() runs their mainloop... maybe we should deprecate it and get it removed in 0.11 and add a gst_bus_poll_full() that takes a main context as argument?
Comment 7 Tim-Philipp Müller 2010-07-16 10:09:53 UTC
I wouldn't mind if we  deprecated it, but I'm not really sure if a _full() variant with main context is a better idea, because it doesn't solve the fundamental problem with this kind of function and still encourages bad code design which *will* lead to obscure bugs in corner cases.
Comment 8 Sebastian Dröge (slomo) 2010-07-16 16:53:32 UTC
Ok, then we should remove gst_bus_poll() in 0.11 ;)
Comment 9 David Schleef 2010-09-05 23:42:39 UTC
Created attachment 169534 [details] [review]
deprecate gst_bus_poll()

This patch deprecates gst_bus_poll() and replaces it in the tests/examples with gst_bus_timed_pop_filtered().

You'd think we would have avoided using a function in examples that is documented as "You should never use this function, since it is pure evil."

The tests pass, but I'm not entirely clear on the consequences of simply replacing gst_bus_poll() as I did.  Hopefully someone else can comment.
Comment 10 Sebastian Dröge (slomo) 2010-09-06 08:16:15 UTC
I don't really think deprecating it now is a good idea. It's used in a lot of applications and getting a "wait for messages but let the mainloop continue" manually everywhere is not that easy.
Comment 11 Stefan Sauer (gstreamer, gtkdoc dev) 2011-11-28 11:07:41 UTC
We still have gst_bus_poll in 0.11.