From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <vdavydov.dev@gmail.com>
Date: Thu, 18 Apr 2019 21:23:48 +0300
From: Vladimir Davydov <vdavydov.dev@gmail.com>
Subject: Re: [tarantool-patches] Re: [PATCH] memtx: cancel checkpoint thread
 at exit
Message-ID: <20190418182348.dsdvwqhhhwpqnw4j@esperanza>
References: <8278bbd0a73bbd0e18a4e8633eb0ad71f2ffa3f7.1555601315.git.vdavydov.dev@gmail.com>
 <20190418171551.5zfqurxnyrdkwxjj@esperanza>
 <20190418172455.GH13022@chai>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20190418172455.GH13022@chai>
To: Konstantin Osipov <kostja@tarantool.org>
Cc: tarantool-patches@freelists.org
List-ID: <tarantool-patches.dev.tarantool.org>

On Thu, Apr 18, 2019 at 08:24:55PM +0300, Konstantin Osipov wrote:
> * Vladimir Davydov <vdavydov.dev@gmail.com> [19/04/18 20:19]:
> > On Thu, Apr 18, 2019 at 06:46:54PM +0300, Vladimir Davydov wrote:
> > > +static void
> > > +checkpoint_cancel(struct checkpoint *ckpt)
> > > +{
> > > +	/*
> > > +	 * Cancel the checkpoint thread if it's running and wait
> > > +	 * for it to terminate so as to eliminate the possibility
> > > +	 * of use-after-free.
> > > +	 */
> > > +	if (ckpt->cord.id != 0 &&
> > > +	    tt_pthread_cancel(ckpt->cord.id) != ESRCH)
> > > +		tt_pthread_join(ckpt->cord.id, NULL);
> > > +	checkpoint_delete(ckpt);
> > > +}
> > 
> > Oops, turned out this check isn't quite correct as we don't reset
> > cord.id after joining the thread. As a result, we might call
> > pthread_cancel on an already joined thread, which leads to a crash
> > (caught by Travis CI):
> 
> I would reset cord.id instead.

Umm, no - the flag is already there and is used for the same purpose so
I don't see any point in resetting cord.id. Anyway, it's a bikeshed.
Pushed as is to master.