Re: [PATCH 3/4] run-command: allow aborting async code prematurely

[Jeff King <peff@peff.net>]

On Fri, Apr 01, 2011 at 11:36:53AM +0200, Erik Faye-Lund wrote:

> > 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.

> 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.

> ... 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