From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Date: Thu, 18 Apr 2019 21:23:48 +0300 From: Vladimir Davydov 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 Cc: tarantool-patches@freelists.org List-ID: On Thu, Apr 18, 2019 at 08:24:55PM +0300, Konstantin Osipov wrote: > * Vladimir Davydov [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.