maria-developers team mailing list archive

[Sergey Vojtovich <svoj@xxxxxxxxxxx>]

Hi Sergei,

thanks for your review.

On Wed, Feb 04, 2015 at 07:58:53PM +0100, Sergei Golubchik wrote:
> Hi, Sergey!
> 
> On Feb 02, svoj@xxxxxxxxxxx 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