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 521513 - Firefox crash when using file picker
Firefox crash when using file picker
Status: RESOLVED FIXED
Product: glib
Classification: Platform
Component: gio
2.15.x
Other FreeBSD
: Normal critical
: ---
Assigned To: Alexander Larsson
gtkdev
Depends on:
Blocks:
 
 
Reported: 2008-03-10 06:51 UTC by Pawel Worach
Modified: 2008-03-14 11:03 UTC
See Also:
GNOME target: ---
GNOME version: 2.21/2.22


Attachments
Backtrace (13.89 KB, text/plain)
2008-03-10 06:52 UTC, Pawel Worach
  Details
Remove GIO source when FAM monitor is unreffed (1.66 KB, patch)
2008-03-10 17:52 UTC, Joe Marcus Clarke
none Details | Review
Make sure the g_source is removed when the module is unloaded. (1.34 KB, patch)
2008-03-12 19:33 UTC, Joe Marcus Clarke
none Details | Review

Description Pawel Worach 2008-03-10 06:51:51 UTC
When using the file picker dialog in Firefox 2.0.0.12 the program ends with a SEGV in glib code. This does not happen with glib 2.14.x. Currently using glib-2.15.6 on FreeBSD 8-CURRENT.

Steps to reproduce: "File | Open File... | Cancel"
Comment 1 Pawel Worach 2008-03-10 06:52:53 UTC
Created attachment 106955 [details]
Backtrace
Comment 2 Pawel Worach 2008-03-10 07:15:11 UTC
Also happens with gedit, same repro. steps.
Comment 3 Joe Marcus Clarke 2008-03-10 16:44:36 UTC
The problem is actually for widespread, and occurs in the fam gio backend.  These two traces show what is happening.  When the file chooser is closed, the FAM subscription on fstab is canceled.  Then the monitor is unreffed (lines 1130 and 1131 in gunixmounts.c).  However, the GIO watch on the FAM connection file descriptor is not removed, so as soon as an event comes in to FAM, the callback is triggered in an unreffed object, and the crash occurs.  It seems to me that the GIO watch needs to be removed when the object is unreffed.

(Note: these traces were taken from gnome-terminal which suffers the same problem when the Edit Profile dialog is destroyed.)

  • #0 fam_do_iter_unlocked
    at fam-helper.c line 71
  • #1 _fam_sub_cancel
    at fam-helper.c line 233
  • #2 g_fam_file_monitor_cancel
    at gfamfilemonitor.c line 131
  • #3 g_unix_mount_monitor_finalize
    at gunixmounts.c line 1130
  • #4 IA__g_object_unref
    at gobject.c line 1793
  • #5 g_hal_volume_monitor_finalize
    at ghalvolumemonitor.c line 154
  • #6 IA__g_object_unref
    at gobject.c line 1793
  • #7 g_union_volume_monitor_finalize
    at gunionvolumemonitor.c line 72
  • #8 IA__g_object_unref
    at gobject.c line 1793
  • #9 gtk_file_system_gio_dispose
    at gtkfilesystemgio.c line 477
  • #0 fam_do_iter_unlocked
    at fam-helper.c line 71
  • #1 fam_callback
    at fam-helper.c line 150
  • #2 IA__g_main_context_dispatch
    at gmain.c line 2065
  • #3 g_main_context_iterate
    at gmain.c line 2698
  • #4 IA__g_main_loop_run
    at gmain.c line 2906
  • #5 IA__gtk_main
    at gtkmain.c line 1163
  • #6 main
    at terminal.c line 1338

And this results in an instant crash.  Maybe there needs to be a _fam_sub_shutdown() function in fam-helper.c which removes the GIO watch...?
Comment 4 Joe Marcus Clarke 2008-03-10 17:52:20 UTC
Created attachment 106996 [details] [review]
Remove GIO source when FAM monitor is unreffed
Comment 5 Joe Marcus Clarke 2008-03-10 17:52:55 UTC
I've attached a patch which I believe to be correct.  It does fix the crashes, and I have not observed any other negative effects.
Comment 6 Pawel Worach 2008-03-10 21:08:20 UTC
Yep, the patch makes 2.15.6 not crash, but 2.16.0 seems broken in more horrible ways.

0>pwo@one ~> gedit

[ open file picker ]

(gedit:5061): GLib-GObject-CRITICAL **: g_value_dup_boxed: assertion `G_VALUE_HOLDS_BOXED (value)' failed

(gedit:5061): GLib-GObject-CRITICAL **: g_value_dup_boxed: assertion `G_VALUE_HOLDS_BOXED (value)' failed

(gedit:5061): GLib-GIO-CRITICAL **: g_themed_icon_constructed: assertion `themed->names != NULL && themed->names[0] != NULL' failed

(gedit:5061): GLib-GIO-CRITICAL **: g_themed_icon_constructed: assertion `themed->names != NULL && themed->names[0] != NULL' failed

(gedit:5061): GLib-CRITICAL **: g_strv_length: assertion `str_array != NULL' failed
...
Multiple segmentation faults occurred; can't display error dialog
Comment 7 Joe Marcus Clarke 2008-03-10 21:20:35 UTC
Note: my patch is not perfect.  Even after adding locks in _fam_sub_shutdown(), the FAM file and directory monitors still compete over who will shutdown the FAM connection first.  There is most likely a better way to solve this, but at least my patch quells the crash.  As for glib-2.16.0, that appears to be broken in some other ways.
Comment 8 Pawel Worach 2008-03-10 21:30:41 UTC
Two different backtraces of 2.16.0 without Marcus patch.

This one the picker shows up and paints fully and then gedit crashes, or crashes when cancel is pressed in the picker:

(gdb) bt

Thread 6318608 (LWP 100140)

  • #0 strlen
    from /lib/libc.so.7
  • #1 IA__g_strdup
    at gstrfuncs.c line 91
  • #2 IA__g_strdupv
    at gstrfuncs.c line 2468
  • #3 boxed_proxy_collect_value
    at gboxed.c line 317
  • #4 IA__g_object_new_valist
    at gobject.c line 1015
  • #5 IA__g_object_new
    at gobject.c line 795
  • #6 IA__g_themed_icon_new_from_names
    at gthemedicon.c line 288
  • #7 IA__g_content_type_get_icon
    at gcontenttype.c line 673
  • #8 _g_local_file_info_get
    at glocalfileinfo.c line 1495
  • #9 g_local_file_enumerator_next_file
  • #10 next_files_thread
    at gfileenumerator.c line 567
  • #11 run_in_thread
    at gsimpleasyncresult.c line 613
  • #12 io_job_thread
    at gioscheduler.c line 178
  • #13 g_thread_pool_thread_proxy
    at gthreadpool.c line 265
  • #14 g_thread_create_proxy
    at gthread.c line 635
  • #15 pthread_getprio
    from /lib/libthr.so.3
  • #16 ??

Comment 9 Joe Marcus Clarke 2008-03-11 20:43:08 UTC
More investigation.  The problem stems from the fact that the libgiofam.so module is unloaded when the file chooser is destroyed.  Since the g_source is left active, bad things happen.  Take a look.  When the file chooser is opened:

Breakpoint 3, dlopen (name=0xc95170 "/usr/local/lib/gio/modules/libgiofam.so", 
    mode=1) at /usr/src/libexec/rtld-elf/rtld.c:1880
1880        wlock_release(rtld_bind_lock, lockstate);
(gdb) print obj
$16 = (Obj_Entry *) 0x8075c6200
(gdb) cont
Continuing.

So the FAM module is loaded at 0x8075c6200.  Now, as soon as the chooser is destroyed by clicking on either button:

Breakpoint 4, dlclose (handle=0x8075c6200)
    at /usr/src/libexec/rtld-elf/rtld.c:1742
1742    {
(gdb) bt
  • #0 dlclose
    at /usr/src/libexec/rtld-elf/rtld.c line 1742
  • #1 _g_module_close
    at gmodule-dl.c line 138
  • #2 g_module_close
    at gmodule.c line 580
  • #3 g_io_module_unload_module
    at giomodule.c line 188
  • #4 IA__g_type_module_unuse
    at gtypemodule.c line 227
  • #5 type_data_last_unref_Wm
    at gtype.c line 2063
  • #6 IA__g_type_class_unref
    at gtype.c line 1110
  • #7 g_unix_mount_monitor_finalize
    at gunixmounts.c line 1131

The module is unloaded.  So while my patch sucks, unless the module is not unloaded, I don't see any other way of accommodating this scenario.  Other ideas are certainly welcome.
Comment 10 Matthias Clasen 2008-03-12 19:30:38 UTC
One option would certainly be to make the module resident.

Alex, what is your opinion ?
Comment 11 Joe Marcus Clarke 2008-03-12 19:32:41 UTC
I have come up with a new, better patch.  I don't know why I didn't think of this before.  I simply call the _fam_sub_shutdown() function from the module unload function.  This avoids the races, and also fixes the crash.  I have attached the new patch.
Comment 12 Joe Marcus Clarke 2008-03-12 19:33:52 UTC
Created attachment 107174 [details] [review]
Make sure the g_source is removed when the module is unloaded.
Comment 13 Alexander Larsson 2008-03-14 11:00:42 UTC
The initial patch is wrong, as it doesn't work if you have multiple open monitors. However, the last patch seems very good. Applying it.