GNOME Bugzilla – Bug 562452
Ensure we return G_IO_ERROR_CANCELLED if cancelling g_simple_async_result_run_in_thread
Last modified: 2008-12-10 14:23:38 UTC
Opened file chooser in SciTE (some text editor), deep in directory structure, hold the Ctrl and hitting quickly Arrow Up to get to root dir. It crashed somewhere in the middle. Package: gtk2-2.14.4-3.fc10.i386 Trace caught by bug buddy: System: Linux 2.6.27.5-117.fc10.i686 #1 SMP Tue Nov 18 12:19:59 EST 2008 i686 X Vendor: The X.Org Foundation X Vendor Release: 10503000 Selinux: Enforcing Accessibility: Disabled GTK+ Theme: Nodoka Icon Theme: Crux Memory status: size: 78139392 vsize: 78139392 resident: 22540288 share: 10686464 rss: 22540288 rss_rlim: 18446744073709551615 CPU usage: start_time: 1227793801 rtime: 182 utime: 174 stime: 8 cutime:0 cstime: 0 timeout: 0 it_real_value: 0 frequency: 100 Backtrace was generated from '/usr/bin/SciTE' (no debugging symbols found) [Thread debugging using libthread_db enabled] [New Thread 0xb7f06720 (LWP 3322)] [New Thread 0xb5effb90 (LWP 3324)] 0x00110416 in __kernel_vsyscall ()
+ Trace 210272
Thread 1 (Thread 0xb7f06720 (LWP 3322))
The program is running. Quit anyway (and detach it)? (y or n) [answered Y; input not from terminal] ----------- .xsession-errors --------------------- ** (nm-applet:2775): WARNING **: No connections defined Window manager warning: Attempt to perform window operation 20 on window none when operation 20 on none already in effect Window manager warning: Attempt to perform window operation 20 on window none when operation 20 on none already in effect Window manager warning: Attempt to perform window operation 20 on window none when operation 20 on none already in effect Window manager warning: Attempt to perform window operation 20 on window none when operation 20 on none already in effect Window manager warning: Attempt to perform window operation 20 on window none when operation 20 on none already in effect Window manager warning: Attempt to perform window operation 20 on window none when operation 20 on none already in effect Window manager warning: Attempt to perform window operation 20 on window none when operation 20 on none already in effect Window manager warning: Attempt to perform window operation 20 on window none when operation 20 on none already in effect Window manager warning: Attempt to perform window operation 20 on window none when operation 20 on none already in effect Window manager warning: Attempt to perform window operation 20 on window none when operation 20 on none already in effect Window manager warning: Attempt to perform window operation 20 on window none when operation 20 on none already in effect Window manager warning: Attempt to perform window operation 20 on window none when operation 20 on none already in effect (SciTE:3322): GLib-GObject-WARNING **: instance of invalid non-instantiatable type `(null)' --------------------------------------------------
moving to glib/gio, the fix is supposed to come there.
Created attachment 124268 [details] [review] proposed glib patch for glib/gio;
That is wrong. The func call should set the result of the operation, whether it is a successful result or an error (including a cancelled error). With you patch you either set the cancelled error twice, or the func returns a different result but we override that with an error causing possible leaks or whatnot. Cancellation is inherently racy, and will always be, so you can never be guaranteed that you get a cancelled error if you call g_cancellable_cancel(). What exact problem did you have that caused this patch?
Actually, it looks like there should be no problem with leaking results, so if the error setters were fixed to free existing errors first we could avoid any leaks. And, in theory there is a limited case where we can be race-free, if you're cancelling an async operation and the cancelling happens from the main thread. However, for that to be race free we would need to check is_cancelled in right before calling out to the user in the main thread (i.e. g_simple_async_result_complete), otherwise there is still a race between returning from run_in_thread and calling back on the main thread. The problem is that by the time we get back to g_simple_async_result_complete we no longer have the GCancellable around.
2008-12-10 Alexander Larsson <alexl@redhat.com> Bug 562452 - Ensure we return G_IO_ERROR_CANCELLED if cancelling g_simple_async_result_run_in_thread * gsimpleasyncresult.c: Make g_simple_async_result_run_in_thread check cancellation before calling out to the user in the callback. This means we guarantee reporting cancels of async operations from the main threads, which is probably more in line with what users expect. Note that there are still no such guarantees for cancelling sync operations or cancelling async operation from outside the main thread.
Thanks, it works great.