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 787996 - nautilus segfaults in libtotem-properties/gstreamer code when closing nautilus info
nautilus segfaults in libtotem-properties/gstreamer code when closing nautilu...
Status: RESOLVED FIXED
Product: totem
Classification: Core
Component: Properties page
unspecified
Other Linux
: Normal normal
: ---
Assigned To: General Totem maintainer(s)
General Totem maintainer(s)
Depends on:
Blocks:
 
 
Reported: 2017-09-21 15:15 UTC by Sebastien Bacher
Modified: 2017-09-22 10:40 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
properties: Fix crash when properties are closed fast (980 bytes, patch)
2017-09-21 18:09 UTC, Bastien Nocera
none Details | Review
properties: Fix crash when properties are closed fast (1.06 KB, patch)
2017-09-21 22:01 UTC, Bastien Nocera
none Details | Review
properties: Fix crash when properties are closed fast (1.07 KB, patch)
2017-09-22 10:34 UTC, Bastien Nocera
committed Details | Review

Description Sebastien Bacher 2017-09-21 15:15:03 UTC
Using Ubuntu artful with GNOME 3.26 and gstreamer 1.12.2

- open nautilus
- open the file properties of a mp3
- close the dialog

nautilus segfault, valgrind shows an invalid read in totem/gstreamer code

==1107== Invalid read of size 8
==1107==    at 0x1CEB4F2B: discovered_cb (totem-properties-view.c:287)
==1107==    by 0xDB1EE17: ffi_call_unix64 (in /usr/lib/x86_64-linux-gnu/libffi.so.6.0.4)
==1107==    by 0xDB1E879: ffi_call (in /usr/lib/x86_64-linux-gnu/libffi.so.6.0.4)
==1107==    by 0x70DF798: g_cclosure_marshal_generic (in /usr/lib/x86_64-linux-gnu/libgobject-2.0.so.0.5400.0)
==1107==    by 0x70DEF9C: g_closure_invoke (in /usr/lib/x86_64-linux-gnu/libgobject-2.0.so.0.5400.0)
==1107==    by 0x70F1D5D: ??? (in /usr/lib/x86_64-linux-gnu/libgobject-2.0.so.0.5400.0)
==1107==    by 0x70FA534: g_signal_emit_valist (in /usr/lib/x86_64-linux-gnu/libgobject-2.0.so.0.5400.0)
==1107==    by 0x70FAF4E: g_signal_emit (in /usr/lib/x86_64-linux-gnu/libgobject-2.0.so.0.5400.0)
==1107==    by 0x1D4D912B: discoverer_collect (gstdiscoverer.c:1344)
==1107==    by 0x1D4D9560: discoverer_bus_cb (gstdiscoverer.c:1682)
==1107==    by 0xDB1EE17: ffi_call_unix64 (in /usr/lib/x86_64-linux-gnu/libffi.so.6.0.4)
==1107==    by 0xDB1E879: ffi_call (in /usr/lib/x86_64-linux-gnu/libffi.so.6.0.4)
==1107==    by 0x70DF798: g_cclosure_marshal_generic (in /usr/lib/x86_64-linux-gnu/libgobject-2.0.so.0.5400.0)
==1107==    by 0x70DEF9C: g_closure_invoke (in /usr/lib/x86_64-linux-gnu/libgobject-2.0.so.0.5400.0)
==1107==    by 0x70F1D5D: ??? (in /usr/lib/x86_64-linux-gnu/libgobject-2.0.so.0.5400.0)
==1107==    by 0x70FA534: g_signal_emit_valist (in /usr/lib/x86_64-linux-gnu/libgobject-2.0.so.0.5400.0)
==1107==    by 0x70FAF4E: g_signal_emit (in /usr/lib/x86_64-linux-gnu/libgobject-2.0.so.0.5400.0)
==1107==    by 0x1D732DA1: gst_bus_async_signal_func (in /usr/lib/x86_64-linux-gnu/libgstreamer-1.0.so.0.1202.0)
==1107==    by 0x1D733BD5: ??? (in /usr/lib/x86_64-linux-gnu/libgstreamer-1.0.so.0.1202.0)
==1107==    by 0x5090DE4: g_main_context_dispatch (in /lib/x86_64-linux-gnu/libglib-2.0.so.0.5400.0)
==1107==    by 0x50911AF: ??? (in /lib/x86_64-linux-gnu/libglib-2.0.so.0.5400.0)
==1107==    by 0x509123B: g_main_context_iteration (in /lib/x86_64-linux-gnu/libglib-2.0.so.0.5400.0)
==1107==    by 0x6DD5BEC: g_application_run (in /usr/lib/x86_64-linux-gnu/libgio-2.0.so.0.5400.0)
==1107==    by 0x14FF7B: main (in /usr/bin/nautilus)
==1107==  Address 0x1ff642c0 is 384 bytes inside a block of size 400 free'd
==1107==    at 0x4C30D3B: free (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==1107==    by 0x7103B62: g_type_free_instance (in /usr/lib/x86_64-linux-gnu/libgobject-2.0.so.0.5400.0)
==1107==    by 0x55B79E7: gtk_notebook_forall (gtknotebook.c:4578)
==1107==    by 0x54C4AAD: gtk_container_destroy (gtkcontainer.c:1700)
==1107==    by 0x70DEEB0: g_closure_invoke (in /usr/lib/x86_64-linux-gnu/libgobject-2.0.so.0.5400.0)
==1107==    by 0x70F1ED1: ??? (in /usr/lib/x86_64-linux-gnu/libgobject-2.0.so.0.5400.0)
==1107==    by 0x70FA534: g_signal_emit_valist (in /usr/lib/x86_64-linux-gnu/libgobject-2.0.so.0.5400.0)
==1107==    by 0x70FAF4E: g_signal_emit (in /usr/lib/x86_64-linux-gnu/libgobject-2.0.so.0.5400.0)
==1107==    by 0x56E0CBB: gtk_widget_dispose (gtkwidget.c:12070)
==1107==    by 0x70E5707: g_object_run_dispose (in /usr/lib/x86_64-linux-gnu/libgobject-2.0.so.0.5400.0)
==1107==    by 0x5479A1B: gtk_box_forall (gtkbox.c:2671)
==1107==    by 0x54C4AAD: gtk_container_destroy (gtkcontainer.c:1700)
==1107==    by 0x70DEEB0: g_closure_invoke (in /usr/lib/x86_64-linux-gnu/libgobject-2.0.so.0.5400.0)
==1107==    by 0x70F1ED1: ??? (in /usr/lib/x86_64-linux-gnu/libgobject-2.0.so.0.5400.0)
==1107==    by 0x70FA534: g_signal_emit_valist (in /usr/lib/x86_64-linux-gnu/libgobject-2.0.so.0.5400.0)
==1107==    by 0x70FAF4E: g_signal_emit (in /usr/lib/x86_64-linux-gnu/libgobject-2.0.so.0.5400.0)
==1107==    by 0x56E0CBB: gtk_widget_dispose (gtkwidget.c:12070)
==1107==    by 0x70E5707: g_object_run_dispose (in /usr/lib/x86_64-linux-gnu/libgobject-2.0.so.0.5400.0)
==1107==    by 0x56ECFC8: gtk_window_forall (gtkwindow.c:8503)
==1107==    by 0x54C4AAD: gtk_container_destroy (gtkcontainer.c:1700)
==1107==    by 0x70DEF9C: g_closure_invoke (in /usr/lib/x86_64-linux-gnu/libgobject-2.0.so.0.5400.0)
==1107==    by 0x70F1ED1: ??? (in /usr/lib/x86_64-linux-gnu/libgobject-2.0.so.0.5400.0)
==1107==    by 0x70FA534: g_signal_emit_valist (in /usr/lib/x86_64-linux-gnu/libgobject-2.0.so.0.5400.0)
==1107==    by 0x70FAF4E: g_signal_emit (in /usr/lib/x86_64-linux-gnu/libgobject-2.0.so.0.5400.0)
==1107==    by 0x56E0CBB: gtk_widget_dispose (gtkwidget.c:12070)
==1107==    by 0x56F48D7: gtk_window_dispose (gtkwindow.c:3154)
==1107==    by 0x70E5707: g_object_run_dispose (in /usr/lib/x86_64-linux-gnu/libgobject-2.0.so.0.5400.0)
==1107==    by 0x70DEF9C: g_closure_invoke (in /usr/lib/x86_64-linux-gnu/libgobject-2.0.so.0.5400.0)
==1107==    by 0x70F17D7: ??? (in /usr/lib/x86_64-linux-gnu/libgobject-2.0.so.0.5400.0)
==1107==    by 0x70FA534: g_signal_emit_valist (in /usr/lib/x86_64-linux-gnu/libgobject-2.0.so.0.5400.0)
==1107==  Block was alloc'd at
==1107==    at 0x4C2FB0F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==1107==    by 0x5096538: g_malloc (in /lib/x86_64-linux-gnu/libglib-2.0.so.0.5400.0)
==1107==    by 0x50AE0B5: g_slice_alloc (in /lib/x86_64-linux-gnu/libglib-2.0.so.0.5400.0)
==1107==    by 0x50AE548: g_slice_alloc0 (in /lib/x86_64-linux-gnu/libglib-2.0.so.0.5400.0)
==1107==    by 0x7103865: g_type_create_instance (in /usr/lib/x86_64-linux-gnu/libgobject-2.0.so.0.5400.0)
==1107==    by 0x70E4357: ??? (in /usr/lib/x86_64-linux-gnu/libgobject-2.0.so.0.5400.0)
==1107==    by 0x70E5E04: g_object_new_with_properties (in /usr/lib/x86_64-linux-gnu/libgobject-2.0.so.0.5400.0)
==1107==    by 0x70E6880: g_object_new (in /usr/lib/x86_64-linux-gnu/libgobject-2.0.so.0.5400.0)
==1107==    by 0x1CEB553A: totem_properties_view_new (totem-properties-view.c:383)
==1107==    by 0x1CEB497E: totem_properties_get_pages (totem-properties-main.c:117)
==1107==    by 0x15BD03: ??? (in /usr/bin/nautilus)
==1107==    by 0x15FE05: ??? (in /usr/bin/nautilus)
==1107==    by 0x1F1D4A: ??? (in /usr/bin/nautilus)
==1107==    by 0x5090DE4: g_main_context_dispatch (in /lib/x86_64-linux-gnu/libglib-2.0.so.0.5400.0)
==1107==    by 0x50911AF: ??? (in /lib/x86_64-linux-gnu/libglib-2.0.so.0.5400.0)
==1107==    by 0x509123B: g_main_context_iteration (in /lib/x86_64-linux-gnu/libglib-2.0.so.0.5400.0)
==1107==    by 0x6DD5BEC: g_application_run (in /usr/lib/x86_64-linux-gnu/libgio-2.0.so.0.5400.0)
==1107==    by 0x14FF7B: main (in /usr/bin/nautilus)
Comment 1 Tim-Philipp Müller 2017-09-21 15:33:57 UTC
Line 287 in totem-properties-view.c is:

	gtk_label_set_text (GTK_LABEL (props->priv->label), _(label));

so it looks like the callback is being called after the notebook was destroyed.

Looks like the GstDiscoverer instance was not stopped/destroyed when the widget used in the callback went away.

Looks like a bug in totem properties view at first glance.
Comment 2 Bastien Nocera 2017-09-21 18:09:07 UTC
Created attachment 360209 [details] [review]
properties: Fix crash when properties are closed fast

Cancel the GstDiscoverer process when closing the window.
Comment 3 Bastien Nocera 2017-09-21 18:12:18 UTC
(In reply to Tim-Philipp Müller from comment #1)
> Line 287 in totem-properties-view.c is:
> 
> 	gtk_label_set_text (GTK_LABEL (props->priv->label), _(label));
> 
> so it looks like the callback is being called after the notebook was
> destroyed.
> 
> Looks like the GstDiscoverer instance was not stopped/destroyed when the
> widget used in the callback went away.

It's destroyed, but I'm guessing the async process keeps a reference to itself.

> Looks like a bug in totem properties view at first glance.

If what I say above is true, then it's possible that there's also a bug in GstDiscoverer, as we do unref our only reference to it when closing the window.

Please test and see whether you can reproduce the problem.
Comment 4 Sebastien Bacher 2017-09-21 19:29:26 UTC
The patch seems to fix the segfault but there are still those warnings displayed

* bacon_video_widget_properties_reset: assertion 'props != NULL' failed

which corresponding bt

  • #0 g_log
    at ../../../../glib/gmessages.c line 1399
  • #1 totem_properties_view_set_location
    at ../src/totem-properties-view.c line 369
  • #2 totem_properties_view_new
    at ../src/totem-properties-view.c line 389
  • #3 totem_properties_get_pages
    at ../src/totem-properties-main.c line 117
  • #1 g_return_if_fail_warning
    at ../../../../glib/gmessages.c line 2702
  • #2 discovered_cb
    at ../src/totem-properties-view.c line 296
  • #3 ffi_call_unix64
    at ../src/x86/unix64.S line 76
  • #4 ffi_call
    at ../src/x86/ffi64.c line 525
  • #9 <emit signal ??? on instance 0x5555559477e0 [GstDiscoverer]>
    at ../../../../gobject/gsignal.c line 3447
  • #10 discoverer_collect
    at gstdiscoverer.c line 1344
  • #11 discoverer_bus_cb
    at gstdiscoverer.c line 1682
  • #12 ffi_call_unix64
    at ../src/x86/unix64.S line 76
  • #13 ffi_call
    at ../src/x86/ffi64.c line 525
  • #18 <emit signal message:async-done on instance 0x555555f8acb0 [GstBus]>
    at ../../../../gobject/gsignal.c line 3447
  • #19 gst_bus_async_signal_func

Comment 5 Philip Withnall 2017-09-21 19:31:31 UTC
Review of attachment 360209 [details] [review]:

::: src/totem-properties-view.c
@@ +347,3 @@
 	if (props->priv != NULL) {
+		if (props->priv->disco) {
+			gst_discoverer_stop (props->priv->disco);

I’d also disconnect discovered_cb() from its `discovered` signal, to be absolutely tidy about things.
Comment 6 Sebastien Bacher 2017-09-21 19:33:59 UTC
bugzilla mangled the comment which had several warnings comment, e.g

bacon_video_widget_properties_set_label: assertion 'props != NULL' failed

  • #0 g_log
    at ../../../../glib/gmessages.c line 1399
  • #1 update_audio
    at ../src/totem-properties-view.c line 223
  • #2 discovered_cb
    at ../src/totem-properties-view.c line 311
  • #3 ffi_call_unix64
    at ../src/x86/unix64.S line 76
  • #4 ffi_call
    at ../src/x86/ffi64.c line 525
  • #9 <emit signal ??? on instance 0x5555559477e0 [GstDiscoverer]>
    at ../../../../gobject/gsignal.c line 3447

Comment 7 Sebastien Bacher 2017-09-21 19:34:59 UTC
(the warnings are displayed when opening the properties dialog)
Comment 8 Bastien Nocera 2017-09-21 22:01:41 UTC
Created attachment 360233 [details] [review]
properties: Fix crash when properties are closed fast

Cancel the GstDiscoverer process when closing the window.
Comment 9 Philip Withnall 2017-09-21 22:53:39 UTC
Review of attachment 360233 [details] [review]:

::: src/totem-properties-view.c
@@ +347,3 @@
 	if (props->priv != NULL) {
+		if (props->priv->disco) {
+			g_signal_handlers_disconnect_by_func (object,

s/object/props->priv->disco/, surely?
Comment 10 Sebastien Bacher 2017-09-22 08:58:43 UTC
sorry for the noise but you can ignore my comments from yesterday night about the warnings, when I tested the patch I didn't set the prefix correctly to /usr and it was a side effect of not finding the .ui which was installed in the standard location.

the patch fixes the segfault and no warning is displayed
Comment 11 Bastien Nocera 2017-09-22 10:34:00 UTC
Created attachment 360250 [details] [review]
properties: Fix crash when properties are closed fast

Cancel the GstDiscoverer process when closing the window.
Comment 12 Bastien Nocera 2017-09-22 10:40:03 UTC
Attachment 360250 [details] pushed as 57ceb48 - properties: Fix crash when properties are closed fast