On Sat, Sep 14, 2002 at 10:39:31AM +0100, Philip Martin wrote:
> Branko Čibej <brane@xbc.nu> writes:
>
> I hadn't considered the pool problem, I think we may need to get the
> application to register the cancellation pool first.
that was my first instinct as well, but is there any reason it can't
be passed in with the cancelation function?
> >
> >
> > You only need the one state. Here's the pseudocode:
> >
> > # global state
> > apr_atomic_t svn_cancelled = false
>
> svn_cancelled should be volatile (on a Unix or POSIX system anyway).
> The way the Subversion code is arranged it may well work without being
> volatile, but we should do it properly.
agreed.
> > apr_mutex_t svn_cancellation_mutex
>
> I'm not sure we need, or can use, a mutex.
>
> > apr_pool_t *svn_cancellation_pool = null
> >
> > # public interfaces
> > def svn_async_cancel(pool):
> > svn_cancellation_mutex.lock()
>
> Is apr_mutex_t.lock async-signal safe? The POSIX pthread functions
> are not async-signal safe. Does APR provide stronger guarantees than
> the POSIX pthread functions? While pthread_mutex_lock may be
> async-signal safe on some platforms, relying on that doesn't look
> right.
issues like this make me think we might want to leave the question of
'what do we do in the cancelation check' to the client app, via a
registered callback function. there are cases where it needs to be
thread safe, and in such cases, the client app could compose a
function that does the appropriate locking. there are other cases
where it will need to be called from a signal handler, which means
that it may not be safe to do anything other than setting a flag. if
we can't design something that will work in all cases, maybe we should
punt on it and let the client app figure it out...
> > apr_atoomic_set(svn_canceled, true)
> > if svn_cancellation_pool is not null:
> > svn_cancellation_pool = pool;
> > pool.register_cleanup(lamba x: *x = null, &svn_cancellation_pool)
> > svn_cancellation_mutex.unlock()
>
> I'd do something like
>
> void svn_async_pool_initialize (apr_pool_t *pool)
> {
> svn_cancellation_pool = pool;
> }
>
> void svn_async_cancel (void)
> {
> svn_cancelled = TRUE;
> }
>
> void svn_async_clear (void)
> {
> svn_cancelled = FALSE;
> }
>
> svn_boolean_t svn_async_is_cancelled (void)
> {
> return svn_cancelled;
> }
>
> The rest as per Brane's mail.
>
> The application must call svn_async_pool_initialize if it is going to
> use svn_async_cancel. Once svn_async_cancel is called the application
> must wait for all threads to return from Subversion functions before
> calling svn_async_clear. If you want to add error checking in
> svn_async_cancel then on a POSIX system write (not stdio) and _exit
> are async-signal safe
>
> if (! svn_cancellation_pool)
> {
> write (stderr, message, sizeof (message));
> _exit();
> }
>
> If the application wants to do fancy mutex stuff in it's signal handler
> then it gets to decide how portable it is. Subversion should restrict
> itself to things that are known to work.
this makes sense to me, but i'm still on the edge concerning the
'cancelation means we cancel ALL subversion calls in ALL running
threads' thing. it certainly simplifies my life (hell, i've got a
patch that's pretty much what you're asking for right now on my
laptop), but it feels wrong, and i'd like some input from other
developers on what they would want this interface to do before we
commit to something.
-garrett
--
garrett rooney Remember, any design flaw you're
rooneg@electricjellyfish.net sufficiently snide about becomes
http://electricjellyfish.net/ a feature. -- Dan Sugalski
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Sat Sep 14 15:58:26 2002