Re: [Tracker] [Patch] New CVS version



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/




[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]