Re: [Tracker] [Patch] New CVS version

[Jamie McCracken <jamiemcc blueyonder co uk>]

Laurent Aguerreche wrote:

> According to what I understood in trackerd.c, the idea is to use
> signal_handler() as a catapult to run an independent and delayed
> do_cleanup() and let signal_handler() finish ASAP to let the interrupted
> thread to continue.

yes thats right.


> So in my opinion, "act.sa_flags = SA_NODEFER;" should be reverted to
> "act.sa_flags = 0" in main(). This will block all the threads as long as
> signal_handler() is running and will avoid any surprise!

no mutexes or g_cond statments must be used in the signal handler hence 
the delayed two phase exit so there should not be any surprises. In any 
event I have set the flags as you stated just in case!

> But I don't see why you delay call to do_cleanup(). The deadlock that I
> pointed was only due to a code never reached which isn't the case
> anymore. Now I only see the segfault I explained in my previous mail
> (i.e. one connection but two concurrent calls to mysql_query()).

If we terminate in the signal handler the interrupted code would not 
have a chance to finish (ie unlocking mutexes, fisnishing DB updates etc 
and hence cause crashes and corruption)

The delayed call as it uses the glib mainloop will only get called when 
the thread handling the signal has finished processing stuff (ie only in 
idle time).

The segfault you mentioned should not happen with this. (I removed the 
call just in case)


> Small things:
> - a typo in tracker_log()?
>   "so now shuting" => "so now shutting"?
> - why tracker_db_clear_temp() is called before any mutex locking? IMHO
> it should be called after mutexes for pooling or I don't understand its
> goal!  :-)
> - I think that a call to tracker_db_thread_end() is missing near the end
> of do_cleanup().


Not needed as the main thread only handles the signals now.

The current issue is the file thread is not always sleeping - the reason 
I think is because in that case its the file thread thats handling the 
signal and not the main thread. In which case the files thread will
never sleep and will therefore hang when we lock on the mutex in 
do_cleanup (files_signal_mutex).

As the threads inherit the signal handling from the main thread, I have 
now added signal blocking to all the created threads using:

sigfillset (&signal_set);
sigprocmask (SIG_BLOCK, &signal_set, NULL);

Ergo the only thread that can handle the signals in now the main thread.

I believe this should fix all outstanding issues and it should therefore 
never crash or hang on exit.

Please test latest CVS to confirm this is the case.


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