Re: permissions (and other) problems

On Sat, Sep 14, 2002 at 04:21:43PM +0100, Philip Martin wrote:
> Garrett Rooney <rooneg@isris.pair.com> writes:
>
> > 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.
>
> Or perhaps we don't need to provide a pool, could we use SVN_ERROR_POOL
> via svn_error__get_error_pool?

that sounds promising, but we still need the client to provide some
kind of pool for us to get the error pool out of.

> > that was my first instinct as well, but is there any reason it can't
> > be passed in with the cancelation function?
>
> The hard part is guaranteeing memory visibility. After the
> cancellation pool pointer gets changed by the asynchronous signal
> handler you need to ensure that when the pointer next gets accessed
> synchronously the change will be visible. When the cancelled flag
> first indicates that cancellation is required, the cancellation pool
> pointer must have the right value. On lots of systems 'volatile' is
> enough to do this for a pointer. Further, if the Subversion code has
> the pointer in one compilation unit, and it gets accessed from other
> compilation units via function calls it may work even without
> 'volatile'. However, to *guarantee* that it works, without having to
> worry about compiler or CPU optimisations, is hard.

ack, that does open a can of worms, doesn't it.

> Of course having the client register an "is cancellation required"
> callback that gets called synchronously, rather than providing an
> asyncronous "set cancellation" function, moves all these problems out
> of the library into the client. The client is then free to abuse the
> rules as it sees fit!

i'm becoming more and more convinced this is the correct approach.

something like this might solve the problem:

typedef svn_error_t * (*svn_cancelation_handler_t) (void *cancel_baton);

void svn_set_cancelation_handler (svn_cancelation_handler_t cancel_handler,
                                  void *cancel_baton);

then SVN_ERR is something like:

#define SVN_ERR(expr) \
  do { \
    svn_error_t *svn_err__temp = SVN_NO_ERROR; \
    if (svn__cancelation_handler) \
      { \
        svn_err__tmp = svn__cancelation_handler (svn__cancelation_baton); \
        if (svn_err__tmp) \
          return svn_err__tmp; \
      } \
    svn_err__temp = (expr); \
    if (svn_err__temp) \
      return svn_err__temp; \
  } while (0)

(with similar things happening in SVN_ERR_W and svn_error_clear_all)

the client app's cancelation handler is responsible for creating the
SVN_ERR_CANCELED error, out of a pool in it's cancelation baton.

the problems of thread safety, async signal safety, and anything else
is conveniently pushed up into the client application, which knows
enough about the environment it will run in to solve it in a robust
manner.

svn__cancelation_handler and svn__cancelation_baton would be declared
volatile, and we'd require that svn_set_cancelation_handler be called
before any subversion function that the app wants to be able to be
canceled, and that it not be called while other subversion functions
are happening (so that svn__cancelation_handler and
svn__cancelation_baton don't have to be protected by a mutex).

-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