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 734035 - gedit hangs up when there's no GSettings key in the registry
gedit hangs up when there's no GSettings key in the registry
Status: RESOLVED FIXED
Product: glib
Classification: Platform
Component: gsettings
unspecified
Other All
: Normal normal
: ---
Assigned To: Allison Karlitskaya (desrt)
gtkdev
Depends on:
Blocks:
 
 
Reported: 2014-07-31 10:16 UTC by LRN
Modified: 2014-07-31 10:47 UTC
See Also:
GNOME target: ---
GNOME version: ---

Attachments
Ensure critial sections are released before returning (1.42 KB, patch)
2014-07-31 10:16 UTC, LRN 		needs-work Details | Review
Ensure critial sections are released before returning (1.44 KB, patch)
2014-07-31 10:23 UTC, LRN 		reviewed Details | Review
Ensure critial sections are released before returning (1.45 KB, patch)
2014-07-31 10:40 UTC, LRN 		committed Details | Review

Description LRN 2014-07-31 10:16:38 UTC 
This happens because watch_add_notify() enters the critical section, but does
not release it when returning. As a result, the watch thread remains blocked,
and later on gsettings finalizer hangs up waiting for the watch thread to die.
Comment 1 LRN 2014-07-31 10:16:41 UTC 
Created attachment 282132 [details] [review]
Ensure critial sections are released before returning
Comment 2 Ignacio Casal Quinteiro (nacho) 2014-07-31 10:19:19 UTC 
Review of attachment 282132 [details] [review]:

See the comment

::: gio/gregistrysettingsbackend.c
@@ +1757,3 @@
+  if (cache_node == NULL || cache_node->data == NULL)
+    {
+      LeaveCriticalSection (self->cache_lock);

the point of g_return_val_if_fail is to also get a warning that we should not reach here. Use g_return_val_if_reached
Comment 3 LRN 2014-07-31 10:23:24 UTC 
Created attachment 282133 [details] [review]
Ensure critial sections are released before returning

v2: use "g_return_val_if_reached (FALSE);" instead of "return FALSE;"
Comment 4 Ignacio Casal Quinteiro (nacho) 2014-07-31 10:24:20 UTC 
Review of attachment 282133 [details] [review]:

Looks good to me. Let's get an ack from a glib dev.
Comment 5 Emmanuele Bassi (:ebassi) 2014-07-31 10:26:24 UTC 
I'd probably use:

  g_warn_if_reached ();
  return FALSE;

instead, since that one does not get elided by defining G_DISABLE_CHECKS.
Comment 6 LRN 2014-07-31 10:40:37 UTC 
Created attachment 282134 [details] [review]
Ensure critial sections are released before returning

v3: Use "g_warn_if_reached (); return FALSE"
Comment 7 LRN 2014-07-31 10:43:05 UTC 
Backtrace (of the main thread hanging in gsettings finalizer):

+ Trace 233899

  • #0 ntdll!ZwWaitForSingleObject
    from C:\Windows\system32\ntdll.dll
  • #1 ntdll!ZwWaitForSingleObject
    from C:\Windows\system32\ntdll.dll
  • #2 WaitForSingleObjectEx
    from C:\Windows\syswow64\KernelBase.dll
  • #3 ??
  • #4 WaitForSingleObjectEx
    from C:\Windows\syswow64\kernel32.dll
  • #5 WaitForSingleObject
    from C:\Windows\syswow64\kernel32.dll
  • #6 watch_remove_notify
    at gregistrysettingsbackend.c line 1826
  • #7 g_registry_backend_unsubscribe
    at gregistrysettingsbackend.c line 1908
  • #8 g_settings_finalize
    at gsettings.c line 582
  • #9 g_object_unref
    at gobject.c line 3170
  • #10 modeline_parser_apply_modeline
    at ../gedit-8ae32ef21204ac4913ea047e0daafb624d2beb86/plugins/modelines/modeline-parser.c line 883
  • #11 gedit_modeline_plugin_activate
    at ../gedit-8ae32ef21204ac4913ea047e0daafb624d2beb86/plugins/modelines/gedit-modeline-plugin.c line 161
  • #12 peas_extension_set_foreach
    at ../../libpeas-1.10.0/libpeas/peas-extension-set.c line 600
  • #13 gedit_view_realize
    at ../gedit-8ae32ef21204ac4913ea047e0daafb624d2beb86/gedit/gedit-view.c line 599
  • #14 g_closure_invoke
    at gclosure.c line 768
  • #15 signal_emit_unlocked_R
    at gsignal.c line 3483
  • #16 g_signal_emit_valist
    at gsignal.c line 3309
  • #17 g_signal_emit
    at gsignal.c line 3365
  • #18 gtk_widget_realize
    at ../../gtk+-09a36b1/gtk/gtkwidget.c line 5508
  • #19 gtk_widget_map
    at ../../gtk+-09a36b1/gtk/gtkwidget.c line 5043
  • #20 gtk_scrolled_window_forall
    at ../../gtk+-09a36b1/gtk/gtkscrolledwindow.c line 1738
  • #21 gtk_container_forall
    at ../../gtk+-09a36b1/gtk/gtkcontainer.c line 2190
  • #22 gtk_container_map
    at ../../gtk+-09a36b1/gtk/gtkcontainer.c line 3450
  • #23 gtk_scrolled_window_map
    at ../../gtk+-09a36b1/gtk/gtkscrolledwindow.c line 3105
  • #24 _g_closure_invoke_va
    at gclosure.c line 831
  • #25 g_signal_emit_valist
    at gsignal.c line 3218
  • #26 g_signal_emit
    at gsignal.c line 3365
  • #27 gtk_widget_map
    at ../../gtk+-09a36b1/gtk/gtkwidget.c line 5045
  • #28 gtk_overlay_forall
    at ../../gtk+-09a36b1/gtk/gtkoverlay.c line 528
  • #29 gtk_container_forall
    at ../../gtk+-09a36b1/gtk/gtkcontainer.c line 2190
  • #30 gtk_container_map
    at ../../gtk+-09a36b1/gtk/gtkcontainer.c line 3450
  • #31 gtk_overlay_map
    at ../../gtk+-09a36b1/gtk/gtkoverlay.c line 450
  • #32 _g_closure_invoke_va
    at gclosure.c line 831
  • #33 g_signal_emit_valist
    at gsignal.c line 3218
  • #34 g_signal_emit
    at gsignal.c line 3365
  • #35 gtk_widget_map
    at ../../gtk+-09a36b1/gtk/gtkwidget.c line 5045
  • #36 gtk_box_forall
    at ../../gtk+-09a36b1/gtk/gtkbox.c line 2574
  • #37 gtk_container_forall
    at ../../gtk+-09a36b1/gtk/gtkcontainer.c line 2190
  • #38 gtk_container_map
    at ../../gtk+-09a36b1/gtk/gtkcontainer.c line 3450
  • #39 _g_closure_invoke_va
    at gclosure.c line 831
  • #40 g_signal_emit_valist
    at gsignal.c line 3218
  • #41 g_signal_emit
    at gsignal.c line 3365
  • #42 gtk_widget_map
    at ../../gtk+-09a36b1/gtk/gtkwidget.c line 5045
  • #43 gtk_widget_set_child_visible
    at ../../gtk+-09a36b1/gtk/gtkwidget.c line 10547
  • #44 gtk_notebook_real_switch_page
    at ../../gtk+-09a36b1/gtk/gtknotebook.c line 6736
  • #45 gedit_notebook_switch_page
    at ../gedit-8ae32ef21204ac4913ea047e0daafb624d2beb86/gedit/gedit-notebook.c line 240
  • #46 g_closure_invoke
    at gclosure.c line 768
  • #47 signal_emit_unlocked_R
    at gsignal.c line 3591
  • #48 g_signal_emit_valist
    at gsignal.c line 3309
  • #49 g_signal_emit
    at gsignal.c line 3365
  • #50 gtk_notebook_switch_page
    at ../../gtk+-09a36b1/gtk/gtknotebook.c line 6778
  • #51 gtk_notebook_real_insert_page
    at ../../gtk+-09a36b1/gtk/gtknotebook.c line 4732
  • #52 gtk_notebook_insert_page
    at ../../gtk+-09a36b1/gtk/gtknotebook.c line 7082
  • #53 gedit_notebook_add_tab
    at ../gedit-8ae32ef21204ac4913ea047e0daafb624d2beb86/gedit/gedit-notebook.c line 519
  • #54 process_create_tab
    at ../gedit-8ae32ef21204ac4913ea047e0daafb624d2beb86/gedit/gedit-window.c line 3180
  • #55 gedit_window_create_tab
    at ../gedit-8ae32ef21204ac4913ea047e0daafb624d2beb86/gedit/gedit-window.c line 3218
  • #56 gedit_app_activate
    at ../gedit-8ae32ef21204ac4913ea047e0daafb624d2beb86/gedit/gedit-app.c line 855
  • #57 _g_closure_invoke_va
    at gclosure.c line 831
  • #58 g_signal_emit_valist
    at gsignal.c line 3218
  • #59 g_signal_emit
    at gsignal.c line 3365
  • #60 g_application_activate
    at gapplication.c line 2011
  • #61 gedit_app_command_line
    at ../gedit-8ae32ef21204ac4913ea047e0daafb624d2beb86/gedit/gedit-app.c line 978
  • #62 ffi_call_win32
    at ../libffi-3.0.13/src/x86/win32.S line 522
  • #63 ffi_call
    at ../libffi-3.0.13/src/x86/ffi.c line 405
  • #64 g_cclosure_marshal_generic_va
    at gclosure.c line 1541
  • #65 _g_closure_invoke_va
    at gclosure.c line 831
  • #66 g_signal_emit_valist
    at gsignal.c line 3218
  • #67 g_signal_emit
    at gsignal.c line 3365
  • #68 g_application_call_command_line
    at gapplication.c line 872
  • #69 g_application_real_local_command_line
    at gapplication.c line 926
  • #70 g_application_run
    at gapplication.c line 2188
  • #71 main
    at ../gedit-8ae32ef21204ac4913ea047e0daafb624d2beb86/gedit/gedit.c line 118 






Comment 8


Emmanuele Bassi (:ebassi)





          2014-07-31 10:43:10 UTC
        



Review of attachment 282134 [details] [review]:

I think this looks good; maybe Ryan wants to pitch in, but since it's the Windows registry backend I think it's pretty safe to say you and nacho are the maintainers. ;-)






Comment 9


LRN





          2014-07-31 10:47:46 UTC
        



Attachment 282134 [details] pushed as 905a8e6 - Ensure critial sections are released before returning