Bug 370491 – [imp_close] crash in CD/DVD Creator: trying to check the prop...

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 370491 - [imp_close] crash in CD/DVD Creator: trying to check the prop...


Summary:	[imp_close] crash in CD/DVD Creator: trying to check the prop...


Status:	RESOLVED FIXED

Product:	evince
Classification:	Core
Component:	backends
Version:	git master
Hardware:	Other All

Importance:	High critical
Target Milestone:	---
Assigned To:	Evince Maintainers
QA Contact:	Evince Maintainers

URL:
Whiteboard:

Duplicates:	371502 375271 375635 380830 381553 382738 387936 390078 394858 397432 400199 403374 404300 413004 430279 432699 432712 432956 433502 434190 436841 436862 437198 439225 439396 440753 441292 445063 449650 450584 457447 462646 462647 483022 (view as bug list)
Depends on:
Blocks:

Reported:	2006-11-04 13:47 UTC by ryan5001sk8
Modified:	2007-10-19 23:05 UTC

See Also:
GNOME target:	---
GNOME version:	2.15/2.16

Attachments
Patch against current SVN trunk (1.36 KB, patch) 2007-01-24 19:49 UTC, palfrey	none	Details \| Review
Improved version of earlier patch (1.85 KB, patch) 2007-01-24 21:41 UTC, palfrey	committed	Details \| Review

Description ryan5001sk8 2006-11-04 13:47:47 UTC

What were you doing when the application crashed?
trying to check the properties of a password protected OOo presentation. 


Distribution: Ubuntu 6.10 (edgy)
Gnome Release: 2.16.1 2006-10-02 (Ubuntu)
BugBuddy Version: 2.16.0

Memory status: size: 82665472 vsize: 0 resident: 82665472 share: 0 rss: 26910720 rss_rlim: 0
CPU usage: start_time: 1162636794 rtime: 0 utime: 561 stime: 0 cutime:528 cstime: 0 timeout: 33 it_real_value: 0 frequency: 0

Backtrace was generated from '/usr/bin/nautilus'

(no debugging symbols found)
Using host libthread_db library "/lib/tls/i686/cmov/libthread_db.so.1".
(no debugging symbols found)
[Thread debugging using libthread_db enabled]
[New Thread -1226574160 (LWP 4412)]
(no debugging symbols found)
0xffffe410 in __kernel_vsyscall ()

+ Trace 82753

Thread 1 (Thread -1226574160 (LWP 4412))

#0 __kernel_vsyscall
#1 __waitpid_nocancel
from /lib/tls/i686/cmov/libpthread.so.0
#2 gnome_gtk_module_info_get
from /usr/lib/libgnomeui-2.so.0
#3 <signal handler called>
#4 imp_close
from /usr/lib/nautilus/extensions-1.0/libevince-properties-page.so
#5 impress_document_get_type
from /usr/lib/nautilus/extensions-1.0/libevince-properties-page.so
#6 g_object_unref
from /usr/lib/libgobject-2.0.so.0
#7 ev_document_factory_get_document
from /usr/lib/nautilus/extensions-1.0/libevince-properties-page.so
#8 nautilus_module_initialize
from /usr/lib/nautilus/extensions-1.0/libevince-properties-page.so
#9 nautilus_property_page_provider_get_pages
from /usr/lib/libnautilus-extension.so.1
#10 fm_directory_view_bump_zoom_level
#11 fm_directory_view_bump_zoom_level
#12 nautilus_clipboard_monitor_emit_changed
#13 nautilus_directory_async_state_changed
#14 nautilus_directory_async_state_changed
#15 nautilus_undo_transaction_unregister_object
#16 nautilus_file_get_volume
#17 fm_directory_view_bump_zoom_level
#18 fm_directory_view_bump_zoom_level
#19 g_cclosure_marshal_VOID__VOID
from /usr/lib/libgobject-2.0.so.0
#20 g_closure_invoke
from /usr/lib/libgobject-2.0.so.0
#21 g_signal_chain_from_overridden
from /usr/lib/libgobject-2.0.so.0
#22 g_signal_emit_valist
from /usr/lib/libgobject-2.0.so.0
#23 g_signal_emit
from /usr/lib/libgobject-2.0.so.0
#24 _gtk_action_emit_activate
from /usr/lib/libgtk-x11-2.0.so.0
#25 gtk_action_activate
from /usr/lib/libgtk-x11-2.0.so.0
#26 g_cclosure_marshal_VOID__VOID
from /usr/lib/libgobject-2.0.so.0
#27 g_closure_invoke
from /usr/lib/libgobject-2.0.so.0
#28 g_signal_chain_from_overridden
from /usr/lib/libgobject-2.0.so.0
#29 g_signal_emit_valist
from /usr/lib/libgobject-2.0.so.0
#30 g_signal_emit
from /usr/lib/libgobject-2.0.so.0
#31 gtk_widget_activate
from /usr/lib/libgtk-x11-2.0.so.0
#32 gtk_menu_shell_activate_item
from /usr/lib/libgtk-x11-2.0.so.0
#33 gtk_menu_shell_append
from /usr/lib/libgtk-x11-2.0.so.0
#34 gtk_menu_reorder_child
from /usr/lib/libgtk-x11-2.0.so.0
#35 _gtk_marshal_BOOLEAN__BOXED
from /usr/lib/libgtk-x11-2.0.so.0
#36 g_value_set_boxed
from /usr/lib/libgobject-2.0.so.0
#37 g_closure_invoke
from /usr/lib/libgobject-2.0.so.0
#38 g_signal_chain_from_overridden
from /usr/lib/libgobject-2.0.so.0
#39 g_signal_emit_valist
from /usr/lib/libgobject-2.0.so.0
#40 g_signal_emit
from /usr/lib/libgobject-2.0.so.0
#41 gtk_widget_get_default_style
from /usr/lib/libgtk-x11-2.0.so.0
#42 gtk_propagate_event
from /usr/lib/libgtk-x11-2.0.so.0
#43 gtk_main_do_event
from /usr/lib/libgtk-x11-2.0.so.0
#44 _gdk_events_init
from /usr/lib/libgdk-x11-2.0.so.0
#45 g_main_context_dispatch
from /usr/lib/libglib-2.0.so.0
#46 g_main_context_check
from /usr/lib/libglib-2.0.so.0
#47 g_main_loop_run
from /usr/lib/libglib-2.0.so.0
#48 gtk_main
from /usr/lib/libgtk-x11-2.0.so.0
#49 POA_Nautilus_MetafileMonitor__init
#50 __libc_start_main
from /lib/tls/i686/cmov/libc.so.6
#51 ??
#0 __kernel_vsyscall

Comment 1 Karsten Bräckelmann 2006-11-06 13:20:20 UTC

*** Bug 371502 has been marked as a duplicate of this bug. ***

Comment 2 Damien Durand 2006-11-14 20:33:22 UTC

*** Bug 375271 has been marked as a duplicate of this bug. ***

Comment 3 André Klapper 2006-11-14 20:43:42 UTC

confirming as per duplicates

Comment 4 Sven 2006-11-24 05:56:45 UTC

you can also create a textfile with *.odp, than try to view the properties and nautilus will crash

Comment 5 Germán Poo-Caamaño 2006-12-02 12:52:36 UTC

*** Bug 381553 has been marked as a duplicate of this bug. ***

Comment 6 Susana 2006-12-05 22:12:18 UTC

*** Bug 382738 has been marked as a duplicate of this bug. ***

Comment 7 Susana 2006-12-20 18:50:42 UTC

*** Bug 387936 has been marked as a duplicate of this bug. ***

Comment 8 Martin Wehner 2006-12-28 03:43:47 UTC

*** Bug 390078 has been marked as a duplicate of this bug. ***

Comment 9 André Klapper 2007-01-11 22:45:10 UTC

*** Bug 394858 has been marked as a duplicate of this bug. ***

Comment 10 André Klapper 2007-01-17 00:25:51 UTC

*** Bug 397432 has been marked as a duplicate of this bug. ***

Comment 11 palfrey 2007-01-24 19:02:05 UTC

*** Bug 400199 has been marked as a duplicate of this bug. ***

Comment 12 palfrey 2007-01-24 19:09:16 UTC

Improved stack trace (nautilus 2.16.1, evince 0.6.1)

+ Trace 104931

#0 imp_close
at document.c line 136
#1 impress_document_finalize
at impress-document.c line 402
#2 g_object_unref
from /usr/lib/libgobject-2.0.so.0
#3 ev_document_factory_get_document
at ev-document-factory.c line 317
#4 ev_properties_get_pages
at ev-properties-main.c line 100
#5 nautilus_property_page_provider_get_pages
at nautilus-property-page-provider.c line 70
#6 append_extension_pages
at fm-properties-window.c line 4238
#7 is_directory_ready_callback
at fm-properties-window.c line 4543
#8 ready_callback_call
at nautilus-directory-async.c line 1237
#9 nautilus_directory_async_state_changed
at nautilus-directory-async.c line 1849
#10 nautilus_directory_call_when_ready_internal
at nautilus-directory-async.c line 1319
#11 vfs_file_call_when_ready
at nautilus-vfs-file.c line 66
#12 nautilus_file_call_when_ready
at nautilus-file.c line 5548

Comment 13 palfrey 2007-01-24 19:49:07 UTC

Created attachment 81107 [details] [review]
Patch against current SVN trunk

The impress handler does a number of things wrong, specifically doing various finalise stuff without checking that various things aren't NULL, and then freeing PangoFontDescriptions in the wrong way. Notably with the later, it *might* have worked until pango started using g_slice_alloc, but only because evince chose the same free'ing mechanism as pango by accident (instead of asking pango to do the freeing for it).

Attached patch fixes all of these. I can no longer reproduce with evince trunk + this patch.

Comment 14 Carlos Garcia Campos 2007-01-24 20:16:19 UTC

Comment on attachment 81107 [details] [review]
Patch against current SVN trunk

>Index: backend/impress/impress-document.c
>===================================================================
>--- backend/impress/impress-document.c	(revision 2248)
>+++ backend/impress/impress-document.c	(working copy)
>@@ -399,14 +399,19 @@
> impress_document_finalize (GObject *object)
> {
>   ImpressDocument *impress_document = IMPRESS_DOCUMENT (object);
>+  if (impress_document == NULL)
>+    	return;

is it really needed? calling g_object_unref() with a null pointer should fail, so I think it's not possible to receive a null here. 

>   g_mutex_free (impress_document->mutex);

this should be checked too I guess. 

>   imp_close (impress_document->imp);

same here

>   imp_delete_context (impress_document->ctx);

and here

>-  g_free (impress_document->pango_ctx);
>-  g_object_unref (G_OBJECT (impress_document->pixmap));
>-  g_object_unref (impress_document->gc);
>+  if (impress_document->pango_ctx)
>+	  pango_font_description_free (impress_document->pango_ctx);
>+  if (impress_document->pixmap)
>+	  g_object_unref (G_OBJECT (impress_document->pixmap));
>+  if (impress_document->gc)
>+	  g_object_unref (impress_document->gc);

these pointers should be set to null after freeing it. Please, add checks for every pointer and set them to null.  

>   G_OBJECT_CLASS (impress_document_parent_class)->finalize (object);
> }

Thanks a lot for the patch :-)

Comment 15 palfrey 2007-01-24 21:41:39 UTC

Created attachment 81123 [details] [review]
Improved version of earlier patch

I've kept in the initial check, mainly because I think I managed to see that scenario at some point in my testing. It could just have been an artefact of compilation with optimisation switched on, but better safe than sorry!

All of the struct members are now checked before being freed/unref'ed, and are all now NULL'ed after the free/unref.

Comment 16 Carlos Garcia Campos 2007-01-24 21:58:49 UTC

Great ;-) Please, commit it. Thanks.

Comment 17 palfrey 2007-01-24 22:12:44 UTC

Love to commit it, but can't. No gnome.org account. Those 18 points you see are mostly the results of doing a whole massive amount of bugsquad stuff over the last month-and-a-half. I've got the permissions to mess around with bugs, but nothing else. Would you mind committing it on my behalf?

Comment 18 Nickolay V. Shmyrev 2007-01-25 09:23:37 UTC

Tom, remember about coding style :) Actually the right fix should go earlier, you should set gerror in impress_document_load and check for document not NULL in ev-properties-page. Finalize checks aren's so helpful. I've committed updated patch now, please test.

Comment 19 palfrey 2007-01-25 11:13:01 UTC

With current SVN trunk, I can no longer cause the crash. The committed version doesn't however set all of the pointers to NULL after freeing them as Carlos suggested in Comment #14, but it still appears to work so far.

Comment 20 Carlos Garcia Campos 2007-01-25 11:30:24 UTC

It's not strictly necessary to set pointers to null, since g_free will check whether the pointer is null before freeing it. It's just a common practice.

Comment 21 Karsten Bräckelmann 2007-02-05 03:15:08 UTC

*** Bug 404300 has been marked as a duplicate of this bug. ***

Comment 22 Karsten Bräckelmann 2007-02-05 03:15:16 UTC

*** Bug 403374 has been marked as a duplicate of this bug. ***

Comment 23 Nickolay V. Shmyrev 2007-02-10 08:01:27 UTC

*** Bug 380830 has been marked as a duplicate of this bug. ***

Comment 24 André Klapper 2007-02-28 16:44:26 UTC

*** Bug 413004 has been marked as a duplicate of this bug. ***

Comment 25 Christian Kirbach 2007-03-13 20:47:47 UTC

*** Bug 375635 has been marked as a duplicate of this bug. ***

Comment 26 palfrey 2007-04-16 15:07:36 UTC

*** Bug 430279 has been marked as a duplicate of this bug. ***

Comment 27 palfrey 2007-04-26 10:22:46 UTC

*** Bug 433502 has been marked as a duplicate of this bug. ***

Comment 28 palfrey 2007-04-27 15:09:47 UTC

*** Bug 432699 has been marked as a duplicate of this bug. ***

Comment 29 palfrey 2007-04-27 15:09:53 UTC

*** Bug 432712 has been marked as a duplicate of this bug. ***

Comment 30 palfrey 2007-04-27 15:15:14 UTC

*** Bug 432956 has been marked as a duplicate of this bug. ***

Comment 31 palfrey 2007-04-28 22:56:17 UTC

*** Bug 434190 has been marked as a duplicate of this bug. ***

Comment 32 palfrey 2007-05-09 11:20:26 UTC

*** Bug 436841 has been marked as a duplicate of this bug. ***

Comment 33 palfrey 2007-05-09 11:20:33 UTC

*** Bug 436862 has been marked as a duplicate of this bug. ***

Comment 34 Pedro Villavicencio 2007-05-09 23:57:26 UTC

*** Bug 437198 has been marked as a duplicate of this bug. ***

Comment 35 Pedro Villavicencio 2007-05-17 19:56:28 UTC

*** Bug 439225 has been marked as a duplicate of this bug. ***

Comment 36 Pedro Villavicencio 2007-05-18 15:10:46 UTC

*** Bug 439396 has been marked as a duplicate of this bug. ***

Comment 37 palfrey 2007-05-25 13:37:21 UTC

*** Bug 440753 has been marked as a duplicate of this bug. ***

Comment 38 Pedro Villavicencio 2007-05-26 01:18:34 UTC

*** Bug 441292 has been marked as a duplicate of this bug. ***

Comment 39 Karsten Bräckelmann 2007-06-21 03:11:37 UTC

*** Bug 449650 has been marked as a duplicate of this bug. ***

Comment 40 André Klapper 2007-06-22 21:27:02 UTC

*** Bug 445063 has been marked as a duplicate of this bug. ***

Comment 41 Pedro Villavicencio 2007-06-24 16:33:57 UTC

*** Bug 450584 has been marked as a duplicate of this bug. ***

Comment 42 André Klapper 2007-07-26 13:10:34 UTC

*** Bug 457447 has been marked as a duplicate of this bug. ***

Comment 43 Martin Wehner 2007-08-02 00:49:30 UTC

*** Bug 462646 has been marked as a duplicate of this bug. ***

Comment 44 Martin Wehner 2007-08-02 00:50:19 UTC

*** Bug 462647 has been marked as a duplicate of this bug. ***

Comment 45 André Klapper 2007-10-19 23:05:38 UTC

*** Bug 483022 has been marked as a duplicate of this bug. ***