Re: [Tracker] [PATCH] Better signals handling

[Jamie McCracken <jamiemcc blueyonder co uk>]

Laurent Aguerreche wrote:
> Hello,
>
> when trackerd receives some signals, signal_handler() is called then
> do_cleanup() and exit().
>
> If you add some sleep() in do_cleanup() you will see that all the
> threads stop at signals trapping! So, now, I propose to use SA_NODEFER
> as flag for sigaction() so other threads will continue to run.

okay sounds good

> But there are still remaining issues. To synchronize threads in
> do_cleanup() some lock_mutex() are called on mutexes placed in
> while-loops. What I dislike there is that those threads will never reach
> their end: process_files_thread() will never call mysql_close() and
> mysql_thread_end() for instance...
> So I decided to make some threads (file_process_thread and
> file_metadata_thread) joinable and to wait their end with
> g_thread_join() after "is_running" was set to FALSE in do_cleanup().

I dont like the joins because they can introduce race conditions. I have 
sorted this by including file_stopped_mutex, metadata_stopped_mutex etc
(it has the same effect as the thread joining minus the race)

> process_user_request_queue_thread() function has a pathological problem:
> it uses g_async_queue_pop() which is blocked until a request come... So
> impossible to end this thread correctly. A "DBUS_SHUTDOWN" action may
> end this thread properly or we could play with _try_pop(), mutexes and
> g_usleep() like in the other functions...

The try_pop and g_usleep were a nasty hack by me! They are bad because 
they wake up tracker every 10ms or so to poll for new data which can 
kill battery life on mobiles.

I have corrected all this by using glib's GCond to sync threads without 
polling or periodically waking up.

I have also added Check mutexes to prevent race conditions.

> If we use a system with "stop" events in queues, we could use the same
> trick for process_files_thread() since it uses g_async_queue_try_pop()
> (that doesn't block) but get data from tracker-inotify.c (for instance).
> Currently inotify events are never stopped but we could stop them then
> put a "stop" in file_process_queue to end process_files_thread().

I have now stopped inotify events

Also added a two phase shutdown process. The Sigaction handler is wrong 
because it stops the current processing in the main thread which could 
result in db update being interrupted or other mutex guarded stuff not 
being unlocked and hence cause a deadlock and hang on termination.

I have added a timer to the handler so that do_cleanup () is called 
100ms after the handler. This allows the main thread time to tody itself 
up and avoid the deadlocks that your patch could create.

The first phase of the shutdown prevents new events being added to the 
existing queues. The second phase coaxes all the threads into a sleep 
state and then we wake em up and terminate them cleanly. Once all the 
threads have been terminated we then exit normally.

Anyway thanks for the patch as it give me the inspiration to fix all 
these :)

Exiting should now be a lot more cleaner in tracker cvs.

--
Mr Jamie McCracken
http://jamiemcc.livejournal.com/