GNOME Bugzilla – Bug 734144
Tracker processes fail to respond to INT or TERM while initialising
Last modified: 2014-08-06 20:33:58 UTC
Patches attached (also in branch sam/signal-handling)
Created attachment 282294 [details] [review] tracker-extract: Use default signal handler for SIGALRM and SIGABRT The ALRM handler was introduced in commit d9d1881c2548e5d6d55fad1e897f8f058ef28696. The alarm() function seems to no longer be used, so we can remove it. The ABRT handler was introduced in commit 3f42a4390d48c6a4840a7bcfce74673ebfd23479. I'm not sure of the reason. Other Tracker processes use the default handler for ABRT, so let's be consistent.
Created attachment 282295 [details] [review] Processes shouldn't install the signal handler until the main loop starts The signal handler we install calls g_main_loop_quit() when SIGTERM or SIGINT are raised, but this is only useful if the loop was started. Sending SIGTERM or SIGINT to a Tracker process while it is starting up fails to stop the process, and triggers the following a warning: GLib-CRITICAL **: g_main_loop_quit: assertion 'loop != NULL' failed Ideally we'd call initialize_signal_handler from an idle once the main loop has started, but installing it just before the loop starts should be a big improvement.
Comment on attachment 282294 [details] [review] tracker-extract: Use default signal handler for SIGALRM and SIGABRT Thanks Sam, yes IIRC, this was all to handle dodgy PDF file extraction which could take a long time.
Comment on attachment 282295 [details] [review] Processes shouldn't install the signal handler until the main loop starts Thanks again, you can feel free to just commit these sort of things in the future ;) Same with the signal handler clean up patch! - Just ask for bug review on major feature branches - I will complain if you break anything anyway :P
cheers Martyn, a review of this is helpful for me because I didn't really know why half of that stuff was actually there ! Merged now.