Re: [PATCH 3/4] run-command: allow aborting async code prematurely
- From
Jeff King <peff@peff.net>
- Date
- Apr 1, 2011, 13:59 UTC
- Message-ID
- <20110401135910.GA1650@sigill.intra.peff.net>
- In-Reply-To
- <AANLkTinAQiuYjs+pxBVKM2-aQNSDd_-CC_fMahasTB=V@mail.gmail.com>
On Fri, Apr 01, 2011 at 11:36:53AM +0200, Erik Faye-Lund wrote:
Show 14 quoted lines
> > diff --git a/run-command.c b/run-command.c
> > index 258c880..f179d2a 100644
> > --- a/run-command.c
> > +++ b/run-command.c
> > @@ -439,6 +439,16 @@ int finish_async(struct async *async)
> > return ret;
> > }
> >
> > +void abort_async(struct async *async)
> > +{
> > +#ifndef WIN32
> > + kill(async->pid, 15);
>
> This doesn't compile unless NO_PTHREADS is set, no?Read the cover letter again. This goes on top of an old version, pre-pthreads. Patch 5/4 modernizes it.
Show 11 quoted lines
> This should probably be
>
> void abort_async(struct async *async)
> {
> #ifdef NO_PTHREADS
> kill(async->pid, 15);
> #else
> pthread_cancel(async->tid)
> #endif
> finish_async(async);
> }Yes, eventually it becomes that, in 5/4.
Show 6 quoted lines
> ... and then us Windows-guys must implement something like pthread_cancel(). > > Or maybe not. Can pthread reliably cancel a thread while making sure > that thread isn't holding a mutex (like some thread-safe malloc > implementations do)? If not, I'm not entirely sure we can even reach > this goal.
Yes, as you figured out in a later email, there are cancellation points. However, I'm not sure it matters for this. Run-command's async code is a very abstract primitive that can even be implemented in a separate process. So async code must not use mutexes or assume it shares memory with the parent. Real threaded code should use pthreads directly (and must be optional, or we alienate non-threaded platforms).
-Peff