Hi Sergei, thanks for your review.
On Wed, Feb 04, 2015 at 07:58:53PM +0100, Sergei Golubchik wrote: > Hi, Sergey! > > On Feb 02, [email protected] wrote: > > revision-id: 7320cbf85ddfaf7ef76c8f03718819377fd14318 > > parent(s): 11deb416fe12ebfd65a1f089c7eb6087c192e2dd > > committer: Sergey Vojtovich > > branch nick: 10.1 > > timestamp: 2015-02-02 15:41:49 +0400 > > message: > > > > MDEV-7177 - Server hangs on shutdown after InnoDB error (main.plugin_loaderr > > fails in buildbot) > > > > There was a race condition in timer functionality of query timeouts. > > > > Race was as following: > > main thread: init_thr_timers() > > timer handler thread: my_thread_init() > > main thread: end_thr_timer()/timer_thread_state= ABORTING > > timer handler thread: timer_thread_state= RUNNING, continue normal op > > main thread: waits indefinitely for timer handler thread to go down > > > > The original idea of the fix is to set RUNNING state in main thread, before > > starting timer handler thread. But it didn't survive further cleanups: > > - removed "timer_thread_state" and used "thr_timer_inited" for this purpose > > - removed unused "timer_thread_running" > > - removed code responisble for "timer handler thread" shutdown > > synchronization, > > use pthread_join() instead. > > That looks ok to me. > > The only concern is - in end_thr_timer(), the old code had a timeout > waiting for a timer thread to die. pthread_join() doesn't. > > I don't know if that's a problem, perhaps not. This is one of questionable aspects of "thr timer". IMHO we either wait or not. Timeout waiting looks like hidding bugs. Anyway all client threads (=all users of this subsystem) should be down at this point. Regards, Sergey _______________________________________________ Mailing list: https://launchpad.net/~maria-developers Post to : [email protected] Unsubscribe : https://launchpad.net/~maria-developers More help : https://help.launchpad.net/ListHelp
