You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Rich Drewes <dr...@interstice.com> on 2002/09/12 03:55:23 UTC
permissions (and other) problems
Hello,
1. What is the right method of setting up permissions for using
Subversion on a local filesystem with local users (Unix environment, of
course)? I have had some serious problems with many different
arrangments.
Right now, I have the repository owned and created by user svn, and all
users and the user svn are in the same group. The svn binaries are *not*
setuid svn (which creates its own set of problems). In order to get
things to work properly I have to set the umask for the users to allow
group write permissions since when users do certain svn operations (like
commits) they create files *in the repository dir* (for example the
db/log.00000000xx files) that are *owned by the user* and these files must
be writable by other users when they do subsequent svn operations. Is
this how things should be set up? I am uncomfortable that all users have
to have g+w in their umask, since it makes not only the repository
modifiable by all (which is inevitable of course) but also any files that
the users creates for other purposes. Also, I'm concerned that these
files in the repository are owned by other users may cause a problem when
some sort of database consolidation takes place and svn tries to delete
those files. Is this a concern?
2. More generally, is using Subversion with local users on the local
filesystem (not remotely via http or whatever) supported very well, or are
the remote access methods the only ones that work reliably? I have had
lots of database lockup problems in the past and I'm not sure they are all
going to go away now that I have the issue in #1 worked out with group
write permission enabled in the users' umasks . . .
Thanks,
Rich
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: permissions (and other) problems
Posted by Philip Martin <ph...@codematters.co.uk>.
mark benedetto king <bk...@inquira.com> writes:
> Right. You could have something like
>
>
> static svn_boolean_t cancelled = FALSE;
That should really be
static volatile sig_atomic_t cancelled = 0;
>
> static void
> siginthandler(int sig)
> {
> cancelled = TRUE;
> }
>
> svn_boolean_t
> is_cancelled() {
> return cancelled;
> }
--
Philip Martin
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: permissions (and other) problems
Posted by Philip Martin <ph...@codematters.co.uk>.
Philip Martin <ph...@codematters.co.uk> writes:
> if (! svn_cancellation_pool)
> {
> write (stderr, message, sizeof (message));
> _exit();
> }
Or, (less drastic!)
if (! svn_cancellation_pool)
{
write (stderr, message, sizeof (message));
svn_cancelled = FALSE;
}
It also occurs to me that the svn_async_cancel does not have to be
called from a signal handler, and so the cancellation mechanism
provides a way for the feedback functions to cause an operation to
abort. At present they have no way of doing that.
--
Philip Martin
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: permissions (and other) problems
Posted by Philip Martin <ph...@codematters.co.uk>.
Garrett Rooney <ro...@electricjellyfish.net> writes:
> > directory cannot be removed. You will need to run 'svn cleanup' to
> > rerun the log file and remove the lock. A log file is a file called
> > "log" in the .svn directory, if it does not exist then a pool cleanup
> > handler should remove the lock.
>
> in that case, should we simply require that the user run 'svn cleanup'
> if that happens,
Yes, I think that would be OK.
> or would it be worthwhile to have us check for such a
> situation and run cleanup ourselves before exiting after the
> cancelation?
It might work, but I don't think it's necessary.
--
Philip Martin
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: permissions (and other) problems
Posted by Garrett Rooney <ro...@electricjellyfish.net>.
Philip Martin wrote:
> Garrett Rooney <ro...@electricjellyfish.net> writes:
>
>
>>another thing that's popped up is that if i control-c out of a long
>>running update, we end up leaving lockfiles all over the place, even
>>though we errored out with SVN_ERR_CANCELED. if someone who has more
>>clue about how the locking in libsvn_wc works could explain how this
>>can happen, i'd appreciate it. should there be an error condition
>>where we leave the working copy locked?
>
>
> If you interrupt the running of a log file, then the lock on that
> directory cannot be removed. You will need to run 'svn cleanup' to
> rerun the log file and remove the lock. A log file is a file called
> "log" in the .svn directory, if it does not exist then a pool cleanup
> handler should remove the lock.
in that case, should we simply require that the user run 'svn cleanup'
if that happens, or would it be worthwhile to have us check for such a
situation and run cleanup ourselves before exiting after the cancelation?
-garrett
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: permissions (and other) problems
Posted by Philip Martin <ph...@codematters.co.uk>.
Garrett Rooney <ro...@electricjellyfish.net> writes:
> another thing that's popped up is that if i control-c out of a long
> running update, we end up leaving lockfiles all over the place, even
> though we errored out with SVN_ERR_CANCELED. if someone who has more
> clue about how the locking in libsvn_wc works could explain how this
> can happen, i'd appreciate it. should there be an error condition
> where we leave the working copy locked?
If you interrupt the running of a log file, then the lock on that
directory cannot be removed. You will need to run 'svn cleanup' to
rerun the log file and remove the lock. A log file is a file called
"log" in the .svn directory, if it does not exist then a pool cleanup
handler should remove the lock.
--
Philip Martin
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: permissions (and other) problems
Posted by Garrett Rooney <ro...@electricjellyfish.net>.
Greg Stein wrote:
> On Tue, Sep 17, 2002 at 09:32:16AM -0400, Garrett Rooney wrote:
>
>>Philip Martin wrote:
>>...
>>
>>>>+
>>>>+ svn_cancel_set_cancel_func (check_cancelled, NULL, svn_pool_create (pool));
>>>>+
>>>>+ signal (SIGINT, cancel);
>>>
>>>APR provides apr_signal in apr_signal.h, should we use that?
>>
>>i'm not sure. signal is theoretically part of the c standard, but for
>>consistency's sake, we should probably use the one from apr. i'll look
>>into it tonight and figure out what to do before committing.
>
>
> "theoretically part of the c standard" just doesn't fly. That's the whole
> reason for APR.
>
> We require ANSI compilers (e.g. gcc), but we don't require ANSI libraries.
> And even if a library is *close* to ANSI, it usually has edge cases that we
> fill in using APR.
sounds like we're using apr_signal then!
;-)
-garrett
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: permissions (and other) problems
Posted by Greg Stein <gs...@lyra.org>.
On Tue, Sep 17, 2002 at 09:32:16AM -0400, Garrett Rooney wrote:
> Philip Martin wrote:
>...
> >>+
> >>+ svn_cancel_set_cancel_func (check_cancelled, NULL, svn_pool_create (pool));
> >>+
> >>+ signal (SIGINT, cancel);
> >
> > APR provides apr_signal in apr_signal.h, should we use that?
>
> i'm not sure. signal is theoretically part of the c standard, but for
> consistency's sake, we should probably use the one from apr. i'll look
> into it tonight and figure out what to do before committing.
"theoretically part of the c standard" just doesn't fly. That's the whole
reason for APR.
We require ANSI compilers (e.g. gcc), but we don't require ANSI libraries.
And even if a library is *close* to ANSI, it usually has edge cases that we
fill in using APR.
Cheers,
-g
--
Greg Stein, http://www.lyra.org/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: permissions (and other) problems
Posted by Michael Wood <mw...@its.uct.ac.za>.
On Tue, Sep 17, 2002 at 08:38:18PM -0400, Garrett Rooney wrote:
>
> On Tuesday, September 17, 2002, at 08:31 PM, Peter Davis wrote:
>
> >-----BEGIN PGP SIGNED MESSAGE-----
> >Hash: SHA1
> >
> >Heh, do you think we could start another thread? :-P
>
> nah, then people would have a chance of finding this in the mail
> archives later... i mean who would guess that the cancelation issue is
> being discussed in a thread entitled 'Re: permissions (and other)
> problems' ;-)
>
> changing the subject now would make entirely too much sense.
Contrary to popular belief, changing the subject is not sufficient to
start a new thread.
See all the In-Reply-To and References headers, e.g.:
References: <79...@electricjellyfish.net>
<87...@codematters.co.uk>
In-Reply-To: <87...@codematters.co.uk>
--
Michael Wood <mw...@its.uct.ac.za>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: permissions (and other) problems
Posted by Garrett Rooney <ro...@electricjellyfish.net>.
On Tuesday, September 17, 2002, at 08:31 PM, Peter Davis wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> Heh, do you think we could start another thread? :-P
nah, then people would have a chance of finding this in the mail
archives later... i mean who would guess that the cancelation issue is
being discussed in a thread entitled 'Re: permissions (and other)
problems' ;-)
changing the subject now would make entirely too much sense.
-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
Re: permissions (and other) problems
Posted by Peter Davis <pe...@pdavis.cx>.
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Heh, do you think we could start another thread? :-P
- --
Peter Davis
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.0.7 (GNU/Linux)
iD8DBQE9h8l6hDAgUT1yirARAhzDAJwIlRdG9z9gAILETWuYlukIrcNohwCgk4kO
+z3ZazAK8gQxYcIAVfBqX+E=
=TqMC
-----END PGP SIGNATURE-----
Re: permissions (and other) problems
Posted by Garrett Rooney <ro...@electricjellyfish.net>.
On Tuesday, September 17, 2002, at 08:22 PM, Philip Martin wrote:
> Garrett Rooney <ro...@electricjellyfish.net> writes:
>
>> the thing that worries me about apr_atomic_t is that if atomic ops
>> aren't available on the platform, it falls back to an implementation
>> using mutexes, which means that apr_atomic_{inc,dec,get,set} wouldn't
>> be safe to use in a signal handler.
>
> So apr_atomic_t is atomic with respect to threads, and not with
> respect to signals, then I'd just use sig_atomic_t. If we restrict
> ourselves to "Subversion" platforms, i.e. ones where neon and/or db4
> can be built, I'd hazard a guess that sig_atomic_t will work :)
sounds good enough to me.
i'll commit this stuff as soon as i figure out why my 'fix' to the pool
lifetime problem that was causing segfaults appears to have broken
'make check'...
this probably means it won't be committed until at least tomorrow,
since i'm about to go out and i doubt i'll find it in the next 5
minutes, so if anyone has any other problems with the latest patch i
posted (other than the apr_signal versus signal thing, which i've fixed
locally), you have about 24 hours to speak up ;-)
-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
Re: permissions (and other) problems
Posted by Philip Martin <ph...@codematters.co.uk>.
Garrett Rooney <ro...@electricjellyfish.net> writes:
> the thing that worries me about apr_atomic_t is that if atomic ops
> aren't available on the platform, it falls back to an implementation
> using mutexes, which means that apr_atomic_{inc,dec,get,set} wouldn't
> be safe to use in a signal handler.
So apr_atomic_t is atomic with respect to threads, and not with
respect to signals, then I'd just use sig_atomic_t. If we restrict
ourselves to "Subversion" platforms, i.e. ones where neon and/or db4
can be built, I'd hazard a guess that sig_atomic_t will work :)
--
Philip Martin
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
RE: Re: Cancelling Subversion operations
Posted by Bill Tutt <ra...@lyra.org>.
> From: mark benedetto king [mailto:bking@inquira.com]
>
> On Wed, Sep 18, 2002 at 10:30:18PM +0200, Sander Striker wrote:
> >
> > Uhm. But we had quite some discussion on how to handle wedged
> repositories.
> > Basically we need to fix the way we open the repository. Can we
please
> > revisit the proposed fixes in those threads first? That would
> effectively
> > solve your problem aswell.
> >
>
> No, we know (or think we can figure out) a way to unwedge a *known
> wedged* repository.
>
> The think is, that's the same as lock-stealing, if you're not
> sure that no one else is using it.
>
Then we need to make sure that's as true as possible using code in
libsvn_fs.
People who don't use libsvn_fs deserve to have locks stolen.
Bill
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: #739: Ensuring ACID in Subversion (aka watcher procecesses are
fun)
Posted by Nuutti Kotivuori <na...@iki.fi>.
Garrett Rooney wrote:
> mark benedetto king wrote:
>> On Thu, Sep 19, 2002 at 03:01:24PM -0400, Garrett Rooney wrote:
>>> speaking of ra_pipe, when do we get to see some more of that code
>>> in the tree? ;-)
[...]
> i just think it would be a 'very good thing (tm)' to have the work
> that's being done on ra_pipe being done in the tree, rather than on
> someone's personal machine where it can easily get lost...
Hear! Hear!
Let us see the code, maybe we can even help making this so very
important piece of code.
:-)
-- Naked
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: #739: Ensuring ACID in Subversion (aka watcher procecesses are
fun)
Posted by Garrett Rooney <ro...@electricjellyfish.net>.
mark benedetto king wrote:
> On Thu, Sep 19, 2002 at 03:01:24PM -0400, Garrett Rooney wrote:
>
>>speaking of ra_pipe, when do we get to see some more of that code in the
>>tree? ;-)
>>
>
>
> Well, it will be much easier to produce piecemeal, (more) easily reviewed
> patches if the XMLRPC-EPI and pipe-management stuff were already committed
> (which they would need to be, for a BDB wrapper to work).
well, you've got commit access to work on ra_pipe... if you're planning
on using xmlrpc-epi for that, then i don't see what's wrong with you
adding that to the tree (either support for us linking against an
installed version or a copy in our tree if that's what you're planning
on doing).
i just think it would be a 'very good thing (tm)' to have the work
that's being done on ra_pipe being done in the tree, rather than on
someone's personal machine where it can easily get lost...
-garrett
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: #739: Ensuring ACID in Subversion (aka watcher procecesses are fun)
Posted by mark benedetto king <bk...@inquira.com>.
On Thu, Sep 19, 2002 at 03:01:24PM -0400, Garrett Rooney wrote:
>
> speaking of ra_pipe, when do we get to see some more of that code in the
> tree? ;-)
>
Well, it will be much easier to produce piecemeal, (more) easily reviewed
patches if the XMLRPC-EPI and pipe-management stuff were already committed
(which they would need to be, for a BDB wrapper to work).
--ben
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: #739: Ensuring ACID in Subversion (aka watcher procecesses are
fun)
Posted by Garrett Rooney <ro...@electricjellyfish.net>.
mark benedetto king wrote:
> On Thu, Sep 19, 2002 at 10:24:19AM -0700, Bill Tutt wrote:
>
>>>You are seriously proposing a root process that kills other processes?
>>>Including my Subversion aware editor which happens to be accessing the
>>>repository? And my debugging session which happens to have reused the
>>>crashed Subversion client process ID? This is a "completely and
>>>utterly robust" solution?
>>>
>>
>>Well, we must do something.
>>
>>Alternative suggestions that do solve the problem are certainly
>>welcomed.
>>
>>Bill
>>
>
>
> Well, what oracle does (AFAICT) is this:
>
> 1.) clients connect to a socket (AF_UNIX preferred, AF_INET okay).
i don't think apr currently supports AF_UNIX, although people have
talked about adding that support.
> 2.) the listening on the socket forks of a child to service
> this connection, which is used for an entire transaction (and
> possibly more than one transactionj).
>
> 3.) the child then reads queries from the client, executes them,
> and writes the results back. If the client dies, the child detects
> this easily, and rollsback the outstanding transaction, releases
> locks, and exits.
>
> The code for this child process is extremely thoroughly QAed
> so that it, itself, is unlikely to fail without releasing the
> locks that it holds. Further, the listener mentioned in (2)
> can detect when this (extremely rare) failure happens, and DTRT.
>
> I think this may be the easiest safe strategy (though it does
> require a daemon) (which might be auto-started, as mentioned
> before).
i would very much prefer that it be auto started... having to configure
a daemon for a local repository is just too much to ask from users.
> It is easy to only start one instance of the daemon: only one
> process can bind to a particular socket at once.
>
> I don't think it needs to be setuid; it doesn't matter who owns
> the daemon (any local user with write access can own the daemon).
> One could argue that there are some security problems with this
> approach (a local user could create a hostile server to attempt
> to exploit a client-side vulnerability). If this is a concern,
> then make the repo owned by svn and mode 600 and start the
> daemon ahead of time, or make a setuid daemon-starter.
>
> Of course, this requires that an API be marshalled
> into flattened queries" and responses so that this communication
> can take place over the socket connection.
>
>>From a QA standpoint it might be easiest to do it by making
> an RPC-style wrapper around the BDB API. We can even use
> the xmlrpc-epi library for marshalling of the requests;
> I've done the majority of the work necessary for this already
> for ra_pipe; I even have an IDL compiler. :-)
>
> Unless there are objections, I can probably throw this together
> in a day or two.
i'd love to see this. i'm not positive it's the best plan, but it
sounds like a viable option to provide if nothing else.
speaking of ra_pipe, when do we get to see some more of that code in the
tree? ;-)
-garrett
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: #739: Ensuring ACID in Subversion (aka watcher procecesses are fun)
Posted by Justin Erenkrantz <je...@apache.org>.
On Fri, Sep 20, 2002 at 01:50:16PM -0400, mark benedetto king wrote:
> There are several different possibilities. One approach that works well
> for database access is to use a "connection pool". When a thread wants
apr_reslist_* in apr-util is meant for exactly this.
I haven't used it, but I know that is what this is for. -- justin
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: #739: Ensuring ACID in Subversion (aka watcher procecesses are fun)
Posted by mark benedetto king <bk...@inquira.com>.
On Fri, Sep 20, 2002 at 06:27:31PM +0100, Philip Martin wrote:
> mark benedetto king <bk...@Inquira.Com> writes:
>
> > In effect, ra_dav -> NetBDB would *leverage* the MPM features of
> > apache; the fewer times apache forks, the fewer NetBDB services
> > would be required.
>
> Really, I'd have thought you need at least one NetBDB per thread. Are
> you proposing that threads share a NetBDB connection? Assuming you
> really mean one per thread, how often does Apache create and destroy
> threads?
>
> Using one NetBDB per thread will dramatically increase the number of
> proceses, which kind of negates any benefits of using threads instead
> of processes.
>
There are several different possibilities. One approach that works well
for database access is to use a "connection pool". When a thread wants
to opens a connection, it first looks in the pool to see if any
previously established connections are available. If not, it creates
a new one (unless the total number of outstanding connections has
exceeded a configurable threshold, in which case it blocks until
a new connection becomes available). In any case, it uses the connection
it obtains, and when done, places it into the list of available
connections. Frequently this list has a configurable maximum size;
if more than X connections are already available, this one is
simply closed.
Another mechanism would be to share connections between threads. This
requires significantly more synchronization than the above approach, so
at high levels of concurrency, performance will degrade significantly.
Blending these two approaches is another possibility (i.e., share
N connections between M threads, M > N).
--ben
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: #739: Ensuring ACID in Subversion (aka watcher procecesses are fun)
Posted by Philip Martin <ph...@codematters.co.uk>.
mark benedetto king <bk...@Inquira.Com> writes:
> In effect, ra_dav -> NetBDB would *leverage* the MPM features of
> apache; the fewer times apache forks, the fewer NetBDB services
> would be required.
Really, I'd have thought you need at least one NetBDB per thread. Are
you proposing that threads share a NetBDB connection? Assuming you
really mean one per thread, how often does Apache create and destroy
threads?
Using one NetBDB per thread will dramatically increase the number of
proceses, which kind of negates any benefits of using threads instead
of processes.
--
Philip Martin
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: #739: Ensuring ACID in Subversion (aka watcher procecesses are fun)
Posted by mark benedetto king <bk...@inquira.com>.
On Fri, Sep 20, 2002 at 10:57:47AM +0100, Philip Martin wrote:
>
> A standard Unix fork/exec server.
Right.
>
> > The code for this child process is extremely thoroughly QAed
>
> That's a red herring, all Subversion code is reviewed. Expecting, or
> relying on, one part to be "thoroughly QAed" doesn't really help.
>
You're right, but it would still be nice if the server were small
enough to be easy to review completely, so that it is unlikely
to be crashing on its own. In particular, apache + mod_dav +
mod_dav_svn + libsvn_fs do not really meet this requirement, IMO.
> > so that it, itself, is unlikely to fail without releasing the
> > locks that it holds. Further, the listener mentioned in (2)
> > can detect when this (extremely rare) failure happens, and DTRT.
> >
This feature means that even if the small, simple server does
crash, things will be cleaned up. We're not, as you suggest,
"relying on" the stability of the service.
> > I think this may be the easiest safe strategy (though it does
> > require a daemon) (which might be auto-started, as mentioned
> > before).
>
> What worries me about this proposal is the performance impact on
> mod_dav_svn. We already have a sophisticated server, Apache, where
> the fork/exec stuff has been abstracted into the MPM modules. Adding
> this new server imposes the fork/exec model again. Will this degrade
> (Windows?) servers?
This is a very valid concern, and one I would expect from anyone with
experience with apache. However, my belief is that the two services
(HTTP and "NetBDB") are very different in *connection lifetime*.
HTTP connection lifespan: milliseconds (hopefully)
(ignoring keep-alive optimizations)
This fact is why HTTP *must* have an MPM infrastructure.
NetBDB connection lifetime: lifetime of client program.
ra_local: seconds (or maybe tenths of a second, one day)
ra_dav: lifetime of apache instance
This fact is why NetBDB can probably get away without one.
In effect, ra_dav -> NetBDB would *leverage* the MPM features of
apache; the fewer times apache forks, the fewer NetBDB services
would be required.
Let me reiterate that this is *exactly* the model that Oracle uses;
that doesn't make it right, but they seem to be doing reasonably well
so far.
>
> Aside from the multi-processing issue, I'm also concerned about memory
> usage. Everything into and out of the database now requires memory to
> be allocated in both Apache and the new server. There is also the
> overhead of sending the requests over the local socket.
>
Yes; this is the price you pay for isolation. I'm not sure that, from
a performance standpoint, it makes sense to not pre-fetch additional
records, but, at least in theory, the service would only need enough
memory to hold the largest record in any of the tables. It could
conceivable even stream *this* record to the client. Practically,
I think it makes sense to start with the easiest implementation; with
an API established, the service and the protocol between the client
and service can be optimized at our leisure.
> Now, ra_pipe will require some sort of svnd server, but that should be
> handling ra requests.
>
Actually, ra_pipe itself doesn't require any daemons (though typically
it will be used with ssh, which requires sshd).
Typically, ra_pipe runs a command like "ssh user@remotehost svnpipe".
Then ra_pipe communicates with this sub-process. All of the
daemon bits are taken care of by sshd.
--ben
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: #739: Ensuring ACID in Subversion (aka watcher procecesses are
fun)
Posted by Garrett Rooney <ro...@electricjellyfish.net>.
Sander Striker wrote:
>>From: Garrett Rooney [mailto:rooneg@electricjellyfish.net]
>>Sent: 20 September 2002 15:26
>
>
>>Philip Martin wrote:
>>
>>
>>>>so that it, itself, is unlikely to fail without releasing the
>>>>locks that it holds. Further, the listener mentioned in (2)
>>>>can detect when this (extremely rare) failure happens, and DTRT.
>>>>
>>>>I think this may be the easiest safe strategy (though it does
>>>>require a daemon) (which might be auto-started, as mentioned
>>>>before).
>>>
>>>
>>>What worries me about this proposal is the performance impact on
>>>mod_dav_svn. We already have a sophisticated server, Apache, where
>>>the fork/exec stuff has been abstracted into the MPM modules. Adding
>>>this new server imposes the fork/exec model again. Will this degrade
>>>(Windows?) servers?
>>>
>>>Aside from the multi-processing issue, I'm also concerned about memory
>>>usage. Everything into and out of the database now requires memory to
>>>be allocated in both Apache and the new server. There is also the
>>>overhead of sending the requests over the local socket.
>>>
>>>Now, ra_pipe will require some sort of svnd server, but that should be
>>>handling ra requests.
>>
>>nothing says that this theoretical server has to be used for ALL
>>filesystem access... (well, maybe bill wants it to be, but personally,
>>i don't thing it's a 'hard and fast requirement') we could have it only
>>used for mediating access to the repository for ra_local, and then build
>>enough smarts into the apache module to deal with similar problems (like
>>philip theorized about in another email).
>
>
> Sure it needs to be ALL access. Consider mixed environments where both
> ra_local and ra_dav are used... (and maybe other ra_ layers).
ack, good point, my bad.
-garrett
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
RE: #739: Ensuring ACID in Subversion (aka watcher procecesses are fun)
Posted by Sander Striker <st...@apache.org>.
> From: Garrett Rooney [mailto:rooneg@electricjellyfish.net]
> Sent: 20 September 2002 15:26
> Philip Martin wrote:
>
> >>so that it, itself, is unlikely to fail without releasing the
> >>locks that it holds. Further, the listener mentioned in (2)
> >>can detect when this (extremely rare) failure happens, and DTRT.
> >>
> >>I think this may be the easiest safe strategy (though it does
> >>require a daemon) (which might be auto-started, as mentioned
> >>before).
> >
> >
> > What worries me about this proposal is the performance impact on
> > mod_dav_svn. We already have a sophisticated server, Apache, where
> > the fork/exec stuff has been abstracted into the MPM modules. Adding
> > this new server imposes the fork/exec model again. Will this degrade
> > (Windows?) servers?
> >
> > Aside from the multi-processing issue, I'm also concerned about memory
> > usage. Everything into and out of the database now requires memory to
> > be allocated in both Apache and the new server. There is also the
> > overhead of sending the requests over the local socket.
> >
> > Now, ra_pipe will require some sort of svnd server, but that should be
> > handling ra requests.
>
> nothing says that this theoretical server has to be used for ALL
> filesystem access... (well, maybe bill wants it to be, but personally,
> i don't thing it's a 'hard and fast requirement') we could have it only
> used for mediating access to the repository for ra_local, and then build
> enough smarts into the apache module to deal with similar problems (like
> philip theorized about in another email).
Sure it needs to be ALL access. Consider mixed environments where both
ra_local and ra_dav are used... (and maybe other ra_ layers).
Sander
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: #739: Ensuring ACID in Subversion (aka watcher procecesses are
fun)
Posted by Garrett Rooney <ro...@electricjellyfish.net>.
Philip Martin wrote:
>>so that it, itself, is unlikely to fail without releasing the
>>locks that it holds. Further, the listener mentioned in (2)
>>can detect when this (extremely rare) failure happens, and DTRT.
>>
>>I think this may be the easiest safe strategy (though it does
>>require a daemon) (which might be auto-started, as mentioned
>>before).
>
>
> What worries me about this proposal is the performance impact on
> mod_dav_svn. We already have a sophisticated server, Apache, where
> the fork/exec stuff has been abstracted into the MPM modules. Adding
> this new server imposes the fork/exec model again. Will this degrade
> (Windows?) servers?
>
> Aside from the multi-processing issue, I'm also concerned about memory
> usage. Everything into and out of the database now requires memory to
> be allocated in both Apache and the new server. There is also the
> overhead of sending the requests over the local socket.
>
> Now, ra_pipe will require some sort of svnd server, but that should be
> handling ra requests.
nothing says that this theoretical server has to be used for ALL
filesystem access... (well, maybe bill wants it to be, but personally,
i don't thing it's a 'hard and fast requirement') we could have it only
used for mediating access to the repository for ra_local, and then build
enough smarts into the apache module to deal with similar problems (like
philip theorized about in another email).
-garrett
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: #739: Ensuring ACID in Subversion (aka watcher procecesses are fun)
Posted by Philip Martin <ph...@codematters.co.uk>.
mark benedetto king <bk...@Inquira.Com> writes:
> 1.) clients connect to a socket (AF_UNIX preferred, AF_INET okay).
>
> 2.) the listening on the socket forks of a child to service
> this connection, which is used for an entire transaction (and
> possibly more than one transactionj).
>
> 3.) the child then reads queries from the client, executes them,
> and writes the results back. If the client dies, the child detects
> this easily, and rollsback the outstanding transaction, releases
> locks, and exits.
A standard Unix fork/exec server.
> The code for this child process is extremely thoroughly QAed
That's a red herring, all Subversion code is reviewed. Expecting, or
relying on, one part to be "thoroughly QAed" doesn't really help.
> so that it, itself, is unlikely to fail without releasing the
> locks that it holds. Further, the listener mentioned in (2)
> can detect when this (extremely rare) failure happens, and DTRT.
>
> I think this may be the easiest safe strategy (though it does
> require a daemon) (which might be auto-started, as mentioned
> before).
What worries me about this proposal is the performance impact on
mod_dav_svn. We already have a sophisticated server, Apache, where
the fork/exec stuff has been abstracted into the MPM modules. Adding
this new server imposes the fork/exec model again. Will this degrade
(Windows?) servers?
Aside from the multi-processing issue, I'm also concerned about memory
usage. Everything into and out of the database now requires memory to
be allocated in both Apache and the new server. There is also the
overhead of sending the requests over the local socket.
Now, ra_pipe will require some sort of svnd server, but that should be
handling ra requests.
--
Philip Martin
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Re: #739: Ensuring ACID in Subversion (aka watcher procecesses are fun)
Posted by mark benedetto king <bk...@inquira.com>.
On Thu, Sep 19, 2002 at 10:24:19AM -0700, Bill Tutt wrote:
> >
> > You are seriously proposing a root process that kills other processes?
> > Including my Subversion aware editor which happens to be accessing the
> > repository? And my debugging session which happens to have reused the
> > crashed Subversion client process ID? This is a "completely and
> > utterly robust" solution?
> >
>
> Well, we must do something.
>
> Alternative suggestions that do solve the problem are certainly
> welcomed.
>
> Bill
>
Well, what oracle does (AFAICT) is this:
1.) clients connect to a socket (AF_UNIX preferred, AF_INET okay).
2.) the listening on the socket forks of a child to service
this connection, which is used for an entire transaction (and
possibly more than one transactionj).
3.) the child then reads queries from the client, executes them,
and writes the results back. If the client dies, the child detects
this easily, and rollsback the outstanding transaction, releases
locks, and exits.
The code for this child process is extremely thoroughly QAed
so that it, itself, is unlikely to fail without releasing the
locks that it holds. Further, the listener mentioned in (2)
can detect when this (extremely rare) failure happens, and DTRT.
I think this may be the easiest safe strategy (though it does
require a daemon) (which might be auto-started, as mentioned
before).
It is easy to only start one instance of the daemon: only one
process can bind to a particular socket at once.
I don't think it needs to be setuid; it doesn't matter who owns
the daemon (any local user with write access can own the daemon).
One could argue that there are some security problems with this
approach (a local user could create a hostile server to attempt
to exploit a client-side vulnerability). If this is a concern,
then make the repo owned by svn and mode 600 and start the
daemon ahead of time, or make a setuid daemon-starter.
Of course, this requires that an API be marshalled
into flattened queries" and responses so that this communication
can take place over the socket connection.
Re: #739: Ensuring ACID in Subversion (aka watcher procecesses are fun)
Posted by "Glenn A. Thompson" <gt...@cdr.net>.
Hey,
Pardon my ignorance here but the recovery procedure on sleepycat says this:
"2. It is necessary to recover information after system or application
failure. In this case, recovery processing must be performed on any database
environments that were active at the time of the failure. Recovery processing
involves running the db_recover utility or calling the DB_ENV->open method
with the DB_RECOVER or DB_RECOVER_FATAL flags.
During recovery processing, all database changes made by aborted or unfinished
transactions are undone, and all database changes made by committed
transactions are redone, as necessary. Database applications must not be
restarted until recovery completes. After recovery finishes, the environment
is properly initialized so that applications may be restarted."
I took this to be DB_ENV->open(DB_RECOVER|etc ) == db_recover
svnadmin has a call to recover which is not listed in the help and is
currently compiled out. In this call he aquires an exclusive lock on db.lock.
It has this comment above it.
" /* ### TODO: Get this working with new libsvn_repos API. We need
the repos API to access the lockfile paths and such, but we
apparently don't want the locking that comes along with the repos
API. */
case svnadmin_cmd_recover:"
Is this code being permanently abandoned?
Should the FS api nolonger include a recover function?
"
/* Perform any necessary non-catastrophic recovery on a Berkeley
DB-based Subversion filesystem, stored in the environment PATH. Do
...
any necessary allocation within POOL.
*/
svn_error_t *svn_fs_berkeley_recover (const char *path,
apr_pool_t *pool);"
Just trying to get a handle on this.
Thanks,
gat
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: #739: Ensuring ACID in Subversion (aka watcher procecesses are fun)
Posted by Philip Martin <ph...@codematters.co.uk>.
"Glenn A. Thompson" <gt...@cdr.net> writes:
> > One thing we need to do is ensure that the BDB recovery process is
> > robust. The documentation requires that no other process is using the
> > database when you run recover. At the moment we don't have a way to
> > ensure that. What we need is a filesystem lock in the db directory,
> > such that when it is present svn_repos_open fails. Then the recovery
> > process is
>
> I thought that db.lock was used for this purpose.
No idea, I'm not a BDB expert.
> I thought svnadmin aquires an exclusive lock on for recovery and repos
> grabs a shared lock on open.
svnadmin doesn't do recovery, we run db_recover for that. I was
proposing to add an svnadmin command that does a bit more checking.
This is the behaviour I observe at present.
$ svn import URL path
^C
The database is now "stuck". If I was running another svn command at
the same time, then it hangs. If I start a new command that also
hangs.
$ svn ls URL
This hangs.
Now I run db_recover. It doesn't block. So the recovery runs despite
the hung processes. This violates the recommended recovery procedure
in the BDB documentation. While I can check and kill processes before
running recover, I cannot guarantee that another will not start. Hence
the procedure
$ svnadmin lock repo
Now no new processes can open the database.
$ ps
Kill any existing processes.
$ svnadmin recover repo
This is the only process accessing the database.
--
Philip Martin
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: #739: Ensuring ACID in Subversion (aka watcher procecesses are fun)
Posted by "Glenn A. Thompson" <gt...@cdr.net>.
Hey:
>
> One thing we need to do is ensure that the BDB recovery process is
> robust. The documentation requires that no other process is using the
> database when you run recover. At the moment we don't have a way to
> ensure that. What we need is a filesystem lock in the db directory,
> such that when it is present svn_repos_open fails. Then the recovery
> process is
>
I thought that db.lock was used for this purpose.
I thought svnadmin aquires an exclusive lock on for recovery and repos
grabs a shared lock on open.
gat
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: #739: Ensuring ACID in Subversion (aka watcher procecesses are fun)
Posted by Philip Martin <ph...@codematters.co.uk>.
mark benedetto king <bk...@Inquira.Com> writes:
> > Lock the repository so that new processes fail to open it
> > $ svnadmin lock /path/to/repos
> >
> > Now check for existing processes that are using the DB
> > $ ps
> > $ lsof
> > $ kill xxxx
> > $ kill -9 xxxx
> >
> > Now run BDB recovery and clear the lock
> > $ svnadmin recover /path/to/repository
>
> What happens when svnadmin crashes after obtaining a lock?
It probably means you need to catastrophic BDB recovery.
> You've got a stale lock file.
>
> If you handle stale lock files by rm'ing them, we're back
> into a lock-stealing scenario (how do you really know the
> lock is stale?)
>
> A user can know that no one else is mucking around in his WC.
>
> An administrator frequently isn't quite so sure that no one
> else is working on *exactly the same problem*.
I was assuming that it would be like
$ svnadmin lock repo
$
OK, I can work on fixing this.
$ svnadmin lock repo
svnadmin: error: already locked
$
Oh! Someone else is doing something.
> It's a secure recovery process, but it's a manual recovery process.
> Personally, I don't want to have to run the command sequence above
> every time someone hits ^C on their client. I'd much rather the
> recovery process only be needed in the case of power-outage.
Yes, but we are going to fix the client to handle ^C. I don't want
BDB recovery to run at all, whether manually or automatically, if
someone hits ^C, as that involves other clients failing.
> I think that requiring manual locking, ps-ing, kill-ing, recovering, etc
> does not meet this definition of robustness.
Well, I disagree :)
I think the plans I have seen so far, to automatically kill clients,
are far less desirable. I haven't seen one that looks to be
particularly robust.
BDB recovery won't need to be done that often. If it is we've got
something wrong.
Finally bear in mind that depending on what has happened, BDB recovery
may fail. In which case you need to do catastrophic recovery, which
will almost certainly require manual intervention.
--
Philip Martin
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
RE: Re: #739: Ensuring ACID in Subversion (aka watcher procecesses are fun)
Posted by Bill Tutt <ra...@lyra.org>.
> From: mark benedetto king [mailto:bking@inquira.com]
> On Fri, Sep 20, 2002 at 11:29:12AM +0100, Philip Martin wrote:
> >
> >
> > Now suppose you want also want to run BDB recovery automatically. I
> > probably would not do that myself, but no matter. Can we use Apache
> > to do this? I'm not an Apache expert, but it does have a
controlling
> > process that remains in communication with it's children. Could we
> > provide an Apache module, or a mod_dav_svn directive, that causes
> > Apache to detect children that disappear by dumping core, or
children
> > that hang and become unresponsive? Then Apache could then lock the
> > repository to block new children, terminate any existing mod_dav_svn
> > children and finally run repository recovery.
> >
> > Then, to have a system that automatically recovers a locked database
> > you run Apache and only allow access through ra_dav.
> >
> > Is this possible? Does it satisfy your ACID requirements?
> >
>
> IANAAE, E. :-)
>
> If we only allow access through ra_dav and all of those things can
> be accomplished reliably with apache, then yes, I think it satisfies
> them.
>
Although currently we're not setup to only allow access through ra_dav.
If we did really do that then, yes we'd satisfy the ACID requirements.
Current examples in Subversion where we'd currently violate that
principle:
* svnadmin
* svnlook
etc...
Bill
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: #739: Ensuring ACID in Subversion (aka watcher procecesses are fun)
Posted by mark benedetto king <bk...@inquira.com>.
On Fri, Sep 20, 2002 at 11:29:12AM +0100, Philip Martin wrote:
>
> One thing we need to do is ensure that the BDB recovery process is
> robust. The documentation requires that no other process is using the
> database when you run recover. At the moment we don't have a way to
> ensure that. What we need is a filesystem lock in the db directory,
> such that when it is present svn_repos_open fails. Then the recovery
> process is
>
> Lock the repository so that new processes fail to open it
> $ svnadmin lock /path/to/repos
>
> Now check for existing processes that are using the DB
> $ ps
> $ lsof
> $ kill xxxx
> $ kill -9 xxxx
>
> Now run BDB recovery and clear the lock
> $ svnadmin recover /path/to/repository
What happens when svnadmin crashes after obtaining a lock?
You've got a stale lock file.
If you handle stale lock files by rm'ing them, we're back
into a lock-stealing scenario (how do you really know the
lock is stale?)
A user can know that no one else is mucking around in his WC.
An administrator frequently isn't quite so sure that no one
else is working on *exactly the same problem*.
We'd need a reference-counted lock file, which means we'd need
svnadmin not to exit.
So, you'd have something like:
$ svnadmin lock /path/to/repos
bash(svnadmin)> ps
bash(svnadmin)> lsof
bash(svnadmin)> kill xxxx
bash(svnadmin)> kill -9 xxxx
bash(svnadmin)> svnadmin recover /path/to/repository
bash(svnadmin)> exit
Then, if we're really careful about POSIX flock() semantics,
we could guarantee that
1.) no two svnadmins are running at the same time
2.) no new connections are created after the svnadmin runs
This would probably be a lot less effort than a "NetBDB" implementation,
and obviously would not adversely affect performance, etc. Also, this
functionality would be required for recovery after system crashes.
>
> That provides a secure recovery process, in the face of Subversion
> clients and servers. Obviously a user could write a program that
It's a secure recovery process, but it's a manual recovery process.
Personally, I don't want to have to run the command sequence above
every time someone hits ^C on their client. I'd much rather the
recovery process only be needed in the case of power-outage.
> bypasses svn_repos_open if they have sufficient OS/filesystem access,
> but then they can also use raw BDB calls, normal stdio, or a normal
> editor! If you are concerned about such cases, they are handled by
> the usual OS security measures.
Anyone who does those things deserves what they get. Actually, they
deserve worse. :-)
> At this stage I would argue that we have a "completly and utterly
> robust" system. Once a transaction has been committed it is secure,
> it will never be lost.
Re: #739: Ensuring ACID in Subversion (aka watcher procecesses are fun)
Posted by Philip Martin <ph...@codematters.co.uk>.
"Bill Tutt" <ra...@lyra.org> writes:
> Well, we must do something.
>
> Alternative suggestions that do solve the problem are certainly
> welcomed.
One thing we need to do is ensure that the BDB recovery process is
robust. The documentation requires that no other process is using the
database when you run recover. At the moment we don't have a way to
ensure that. What we need is a filesystem lock in the db directory,
such that when it is present svn_repos_open fails. Then the recovery
process is
Lock the repository so that new processes fail to open it
$ svnadmin lock /path/to/repos
Now check for existing processes that are using the DB
$ ps
$ lsof
$ kill xxxx
$ kill -9 xxxx
Now run BDB recovery and clear the lock
$ svnadmin recover /path/to/repository
That provides a secure recovery process, in the face of Subversion
clients and servers. Obviously a user could write a program that
bypasses svn_repos_open if they have sufficient OS/filesystem access,
but then they can also use raw BDB calls, normal stdio, or a normal
editor! If you are concerned about such cases, they are handled by
the usual OS security measures.
At this stage I would argue that we have a "completly and utterly
robust" system. Once a transaction has been committed it is secure,
it will never be lost.
Now suppose you want also want to run BDB recovery automatically. I
probably would not do that myself, but no matter. Can we use Apache
to do this? I'm not an Apache expert, but it does have a controlling
process that remains in communication with it's children. Could we
provide an Apache module, or a mod_dav_svn directive, that causes
Apache to detect children that disappear by dumping core, or children
that hang and become unresponsive? Then Apache could then lock the
repository to block new children, terminate any existing mod_dav_svn
children and finally run repository recovery.
Then, to have a system that automatically recovers a locked database
you run Apache and only allow access through ra_dav.
Is this possible? Does it satisfy your ACID requirements?
--
Philip Martin
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: #739: Ensuring ACID in Subversion (aka watcher procecesses are fun)
Posted by Philip Martin <ph...@codematters.co.uk>.
Philip Martin <ph...@codematters.co.uk> writes:
> > Alternative suggestions that do solve the problem are certainly
> > welcomed.
>
> It depends on how you define "completely and utterly robust" :-)
>
> Having all the clients receive SVN_ERR_REPOS_RECOVER and print
> "repository /foo/bar/repo requires recovery" and relying on the
> repository administrator running db_recover would do for me.
Having experimented a bit, it appears the clients just hang when there
is a repository problem. At least that's what happens at present. So
it may be difficult to generate an error. That doesn't really affect
my view of the solution, I still think manual intervention is the best
solution. It may be that a hanging client is what alerts the user,
rather than an explicit error message.
> At present, ra_local never loses data once it has been committed, it
> is "completely and utterly robust". All I want is to minimise the
> circumstances that cause SVN_ERR_REPOS_RECOVER. But when it does
> occur I am happy with manual intervention.
--
Philip Martin
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: #739: Ensuring ACID in Subversion (aka watcher procecesses are fun)
Posted by Philip Martin <ph...@codematters.co.uk>.
"Bill Tutt" <ra...@lyra.org> writes:
> > You are seriously proposing a root process that kills other processes?
> > Including my Subversion aware editor which happens to be accessing the
> > repository? And my debugging session which happens to have reused the
> > crashed Subversion client process ID? This is a "completely and
> > utterly robust" solution?
>
> Well, we must do something.
>
> Alternative suggestions that do solve the problem are certainly
> welcomed.
It depends on how you define "completely and utterly robust" :-)
Having all the clients receive SVN_ERR_REPOS_RECOVER and print
"repository /foo/bar/repo requires recovery" and relying on the
repository administrator running db_recover would do for me.
At present, ra_local never loses data once it has been committed, it
is "completely and utterly robust". All I want is to minimise the
circumstances that cause SVN_ERR_REPOS_RECOVER. But when it does
occur I am happy with manual intervention.
--
Philip Martin
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
RE: Re: #739: Ensuring ACID in Subversion (aka watcher procecesses are fun)
Posted by Bill Tutt <ra...@lyra.org>.
> From: Philip Martin [mailto:philip@codematters.co.uk]
>
> "Bill Tutt" <ra...@lyra.org> writes:
>
> > > how does the watcher process kill the other processes? are we
going
> to
> > > install it with the appropriate privs so that it can kill them?
setuid
> > > binaries give me the creeps...
> >
> > Yes, it needs appropriate privs so that it can kill them if
necessary.
> > We can come up with various schemes to tell the client process to
exit
> > ASAP, but I think that the watcher process still needs to be able to
> > forcefully kill the client applications.
>
> You are seriously proposing a root process that kills other processes?
> Including my Subversion aware editor which happens to be accessing the
> repository? And my debugging session which happens to have reused the
> crashed Subversion client process ID? This is a "completely and
> utterly robust" solution?
>
Well, we must do something.
Alternative suggestions that do solve the problem are certainly
welcomed.
Bill
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: #739: Ensuring ACID in Subversion (aka watcher procecesses are fun)
Posted by Philip Martin <ph...@codematters.co.uk>.
"Bill Tutt" <ra...@lyra.org> writes:
> > how does the watcher process kill the other processes? are we going to
> > install it with the appropriate privs so that it can kill them? setuid
> > binaries give me the creeps...
>
> Yes, it needs appropriate privs so that it can kill them if necessary.
> We can come up with various schemes to tell the client process to exit
> ASAP, but I think that the watcher process still needs to be able to
> forcefully kill the client applications.
You are seriously proposing a root process that kills other processes?
Including my Subversion aware editor which happens to be accessing the
repository? And my debugging session which happens to have reused the
crashed Subversion client process ID? This is a "completely and
utterly robust" solution?
--
Philip Martin
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
RE: Re: #739: Ensuring ACID in Subversion (aka watcher procecesses are fun)
Posted by Bill Tutt <ra...@lyra.org>.
> From: Garrett Rooney [mailto:rooneg@electricjellyfish.net]
>
> Bill Tutt wrote:
>
> > If the watcher process detects an exiting registered process that
hasn't
> >
> > deregistered then the datastore is now suspect. The watcher process
> > must now cause all in process transactions to be aborted.
> >
> > This should probably be accomplished by using some asyncrhonous
> > notification + timeout. If the timeout expires before the other
> > remaining processes exit out, then the watcher process may kill the
> > process explicitly.
> >
> > Once all of the registered processes have either exited with a
useful
> > failure message, or forcefully killed, then the watcher is allowed
to
> > recover the datastore.
>
> how does the watcher process kill the other processes? are we going
to
> install it with the appropriate privs so that it can kill them?
setuid
> binaries give me the creeps...
>
Yes, it needs appropriate privs so that it can kill them if necessary.
We can come up with various schemes to tell the client process to exit
ASAP, but I think that the watcher process still needs to be able to
forcefully kill the client applications.
> also, would we have to have the watcher start up at system start?
that
> adds quite a bit of overhead to the process of creating a repository.
> perhaps it would be possible to have the first svn process that tries
to
> access the repository start the watcher...
>
This is the easiest way to kick start the watcher up.
Well, if the svn process itself started the watcher, then yeah, the
binary would need to be setuid. Ick. I'd rather it not be setuid, and
just run in the correct user security context. (Not to mention fun
portability issues if we use setuid.)
Bill
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: #739: Ensuring ACID in Subversion (aka watcher procecesses are
fun)
Posted by Garrett Rooney <ro...@electricjellyfish.net>.
Bill Tutt wrote:
> If the watcher process detects an exiting registered process that hasn't
>
> deregistered then the datastore is now suspect. The watcher process
> must now cause all in process transactions to be aborted.
>
> This should probably be accomplished by using some asyncrhonous
> notification + timeout. If the timeout expires before the other
> remaining processes exit out, then the watcher process may kill the
> process explicitly.
>
> Once all of the registered processes have either exited with a useful
> failure message, or forcefully killed, then the watcher is allowed to
> recover the datastore.
how does the watcher process kill the other processes? are we going to
install it with the appropriate privs so that it can kill them? setuid
binaries give me the creeps...
also, would we have to have the watcher start up at system start? that
adds quite a bit of overhead to the process of creating a repository.
perhaps it would be possible to have the first svn process that tries to
access the repository start the watcher...
-garrett
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
RE: Re: #739: Ensuring ACID in Subversion (aka watcher procecesses are fun)
Posted by Bill Tutt <ra...@lyra.org>.
Well, Windows can just wait for the process to exit using some kind of
WaitForMultipleObject() scheme. We'd probably need some other scheme for
Unix boxes given how useless process APIs are on Unix. i.e. There isn't
a openpid() API that lets you then call waitpid().
Suggestions for an alternate scheme would be appreciated.
Bill
----
Do you want a dangerous fugitive staying in your flat?
No.
Well, don't upset him and he'll be a nice fugitive staying in your flat.
> -----Original Message-----
> From: Glenn A. Thompson [mailto:gthompson@cdr.net]
> Sent: Thursday, September 19, 2002 9:39 AM
> To: Subversion Dev list
> Subject: Re: #739: Ensuring ACID in Subversion (aka watcher
procecesses
> are fun)
>
> >
> > If the watcher process detects an exiting registered process that
hasn't
> >
>
> How is he detecting this?
> Are you thinking about some sort of lease scheme?
> gat
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
> For additional commands, e-mail: dev-help@subversion.tigris.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: #739: Ensuring ACID in Subversion (aka watcher procecesses are fun)
Posted by "Glenn A. Thompson" <gt...@cdr.net>.
>
> If the watcher process detects an exiting registered process that hasn't
>
How is he detecting this?
Are you thinking about some sort of lease scheme?
gat
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
#739: Ensuring ACID in Subversion (aka watcher procecesses are fun)
Posted by Bill Tutt <ra...@lyra.org>.
I've added the following comment to issue #739. For some reason the
comment hasn't showed up on the issues email list yet:
In order to ensure our I of Isolation in Acid, we need to have a
guaranteed way of being able to detect processes that died without
cleaning up after themselves.
One way of doing this is to follow the guidelines in this URL:
http://www.sleepycat.com/docs/ref/env/faq.html and create a watcher
process.
Another would be to move all code that called libsvn_fs into a separate
process.
I think the watcher process is the simplest approach. It'd work
something like this:
When the watcher process starts up, it's assumed the machine is
starting, and you're garunteed that no other programs are accessing
the BDB store. Therefore, the watcher process recovers the store on
startup.
Before libsvn_fs opens the BDB store, it registers the current process
with the watcher process. if this fails, libsvn_fs returns a failure.
(This
code should be a thread safe ref count for the process from libsvn_fs's
end.)
After libsvn_fs closes the BDB store, it notifies the watcher process
that
it has released the BDB store cleanly. (Again, this should be a thread
safe ref count.)
If the watcher process detects an exiting registered process that hasn't
deregistered then the datastore is now suspect. The watcher process
must now cause all in process transactions to be aborted.
This should probably be accomplished by using some asyncrhonous
notification + timeout. If the timeout expires before the other
remaining processes exit out, then the watcher process may kill the
process explicitly.
Once all of the registered processes have either exited with a useful
failure message, or forcefully killed, then the watcher is allowed to
recover the datastore.
Any incoming registration requests must block until the database has
been successfully recovered.
It's almost a shame that the watcher process can't release just the
locks that were owned by the errant process because the process has
exited.
If we could, then life would be much simpler.
FYI,
Bill
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Cancelling Subversion operations
Posted by Bill Tutt <ra...@lyra.org>.
> From: Bill Tutt [mailto:rassilon@lyra.org]
>
[....]
> We must ship something that is completely and utterly robust, if we
> don't we've failed. There is no compromise.
>
Just to further annoy folks, check out this URL at Sleepycat:
http://www.sleepycat.com/docs/ref/env/faq.html
Apparently there are good reasons why ACID stores usually are run out of
process. :)
FYI,
Bill
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Cancelling Subversion operations
Posted by Bill Tutt <ra...@lyra.org>.
> From: Philip Martin [mailto:philip@codematters.co.uk]
>
> "Bill Tutt" <ra...@lyra.org> writes:
>
> > > > No, we know (or think we can figure out) a way to unwedge a
*known
> > > > wedged* repository.
> > > >
> > > > The think is, that's the same as lock-stealing, if you're not
> > > > sure that no one else is using it.
> > >
> > >
http://subversion.tigris.org/servlets/ReadMsg?list=dev&msgId=176926
> > >
> > > And there were more threads on this subject IIRC. One
specifically
> > > dealt with the situation you sketch.
> > >
> >
> > In fact, let me be even clearer in case what I've been saying hasn't
> > come across correctly. The issue outlined by the URL that Sander
> > mentioned above IMNSHO is a 1.0 ship stopping bug.
> >
> > We must be atomic, we must be consistent, we must be isolated, and
we
> > must be durable. Additionally, we must also be reliable. There is no
> > compromise possible. Ctrl-C is but the smallest of the annoying
problems
> > that data stores encounter when trying to ensure durability and
> > reliability. There are hundreds of others.
>
> Maybe I'm missing something, but neither the email referenced above
> nor the Berkeley DB documentation it refers to, appear to solve the
> lock-stealing problem. The suggestion, to always run recover when
> opening the DB is not valid. It requires the Subversion clients to be
> either "a single, usually multithreaded, process" or "a set of
> cooperating processes" and neither of those apply. The Subversion
> clients are a set of independent processes.
>
> It boils down to "the Berkeley DB library itself cannot determine
> whether recovery is required" and so neither can a single Subversion
> client. To make the decision to run recovery requires clients either
> to communicate and interact, or to assume that they are the only
> client.
>
> It looks like compromise is required ;-)
>
No, either we must achieve that goal, or we must re-evaluate our design.
There is no compromise. Even if this means that we need to complete
ra_pipe, so that we have a command line client that uses ra_pipe to talk
to a svnd that then uses BDB.
We must ship something that is completely and utterly robust, if we
don't we've failed. There is no compromise.
The referenced email does explain a possible direction for ensuring that
recovery is always run by the first process that wants to open the BDB
data store.
There are other possible ways for the multiple libsvn_fs clients in
different processes to communicate with each other. I'm not going to go
into any of those possible ways now as I need to head on home.
Again, no compromise is possible here,
Bill
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Cancelling Subversion operations
Posted by Garrett Rooney <ro...@electricjellyfish.net>.
On Wednesday, September 18, 2002, at 09:55 PM, Tom Lord wrote:
>
>
>> ability to at least TRY to clean up after itself when the
>> user hits Ctrl-C
>
>
> That being the only condition under which a process can be expected to
> die, unexpectedly?
that being the only condition under which a process can be expected to
die, unexpectedly, which i personally can protect us from.
-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
Re: Cancelling Subversion operations
Posted by Tom Lord <lo...@regexps.com>.
> ability to at least TRY to clean up after itself when the
> user hits Ctrl-C
That being the only condition under which a process can be expected to
die, unexpectedly?
-t
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Cancelling Subversion operations
Posted by Garrett Rooney <ro...@electricjellyfish.net>.
On Wednesday, September 18, 2002, at 08:25 PM, Philip Martin wrote:
> It boils down to "the Berkeley DB library itself cannot determine
> whether recovery is required" and so neither can a single Subversion
> client. To make the decision to run recovery requires clients either
> to communicate and interact, or to assume that they are the only
> client.
>
> It looks like compromise is required ;-)
like, for example, trying to make it as hard as possible for the user
to shoot themselves in the foot by providing our application with the
ability to at least TRY to clean up after itself when the user hits
Ctrl-C ;-)
-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
Re: Cancelling Subversion operations
Posted by Philip Martin <ph...@codematters.co.uk>.
"Bill Tutt" <ra...@lyra.org> writes:
> > > No, we know (or think we can figure out) a way to unwedge a *known
> > > wedged* repository.
> > >
> > > The think is, that's the same as lock-stealing, if you're not
> > > sure that no one else is using it.
> >
> > http://subversion.tigris.org/servlets/ReadMsg?list=dev&msgId=176926
> >
> > And there were more threads on this subject IIRC. One specifically
> > dealt with the situation you sketch.
> >
>
> In fact, let me be even clearer in case what I've been saying hasn't
> come across correctly. The issue outlined by the URL that Sander
> mentioned above IMNSHO is a 1.0 ship stopping bug.
>
> We must be atomic, we must be consistent, we must be isolated, and we
> must be durable. Additionally, we must also be reliable. There is no
> compromise possible. Ctrl-C is but the smallest of the annoying problems
> that data stores encounter when trying to ensure durability and
> reliability. There are hundreds of others.
Maybe I'm missing something, but neither the email referenced above
nor the Berkeley DB documentation it refers to, appear to solve the
lock-stealing problem. The suggestion, to always run recover when
opening the DB is not valid. It requires the Subversion clients to be
either "a single, usually multithreaded, process" or "a set of
cooperating processes" and neither of those apply. The Subversion
clients are a set of independent processes.
It boils down to "the Berkeley DB library itself cannot determine
whether recovery is required" and so neither can a single Subversion
client. To make the decision to run recovery requires clients either
to communicate and interact, or to assume that they are the only
client.
It looks like compromise is required ;-)
--
Philip Martin
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
RE: Cancelling Subversion operations
Posted by Bill Tutt <ra...@lyra.org>.
> From: Sander Striker [mailto:striker@apache.org]
>
> > From: mark benedetto king [mailto:bking@inquira.com]
> > Sent: 18 September 2002 22:22
>
> > On Wed, Sep 18, 2002 at 10:30:18PM +0200, Sander Striker wrote:
> > >
> > > Uhm. But we had quite some discussion on how to handle wedged
> repositories.
> > > Basically we need to fix the way we open the repository. Can we
> please
> > > revisit the proposed fixes in those threads first? That would
> effectively
> > > solve your problem aswell.
> > >
> >
> > No, we know (or think we can figure out) a way to unwedge a *known
> > wedged* repository.
> >
> > The think is, that's the same as lock-stealing, if you're not
> > sure that no one else is using it.
>
> http://subversion.tigris.org/servlets/ReadMsg?list=dev&msgId=176926
>
> And there were more threads on this subject IIRC. One specifically
> dealt with the situation you sketch.
>
In fact, let me be even clearer in case what I've been saying hasn't
come across correctly. The issue outlined by the URL that Sander
mentioned above IMNSHO is a 1.0 ship stopping bug.
We must be atomic, we must be consistent, we must be isolated, and we
must be durable. Additionally, we must also be reliable. There is no
compromise possible. Ctrl-C is but the smallest of the annoying problems
that data stores encounter when trying to ensure durability and
reliability. There are hundreds of others.
Bill
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
RE: Cancelling Subversion operations
Posted by Sander Striker <st...@apache.org>.
> From: mark benedetto king [mailto:bking@inquira.com]
> Sent: 18 September 2002 22:22
> On Wed, Sep 18, 2002 at 10:30:18PM +0200, Sander Striker wrote:
> >
> > Uhm. But we had quite some discussion on how to handle wedged repositories.
> > Basically we need to fix the way we open the repository. Can we please
> > revisit the proposed fixes in those threads first? That would effectively
> > solve your problem aswell.
> >
>
> No, we know (or think we can figure out) a way to unwedge a *known
> wedged* repository.
>
> The think is, that's the same as lock-stealing, if you're not
> sure that no one else is using it.
http://subversion.tigris.org/servlets/ReadMsg?list=dev&msgId=176926
And there were more threads on this subject IIRC. One specifically
dealt with the situation you sketch.
Sander
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Cancelling Subversion operations
Posted by mark benedetto king <bk...@inquira.com>.
On Wed, Sep 18, 2002 at 10:30:18PM +0200, Sander Striker wrote:
>
> Uhm. But we had quite some discussion on how to handle wedged repositories.
> Basically we need to fix the way we open the repository. Can we please
> revisit the proposed fixes in those threads first? That would effectively
> solve your problem aswell.
>
No, we know (or think we can figure out) a way to unwedge a *known
wedged* repository.
The think is, that's the same as lock-stealing, if you're not
sure that no one else is using it.
--ben
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Cancelling Subversion operations
Posted by Garrett Rooney <ro...@electricjellyfish.net>.
On Sunday, September 22, 2002, at 01:50 PM, Branko Čibej wrote:
> Can't be done, I tried. The only way to unwedge a repository is to run
> recovery in a process that has exclusive access to the repo. If there
> are wedged processes hung there, it won't work. The only way to avoid
> hanging a process is to set a timeout on DB locks and transactions
> (can be done in the DB_CONFIG file). Then you have to guess a correct
> timeout period.
>
> The automatic unwedging thing would work great if we had a DB server
> process. But we don't.
well, given this problem, there are a couple of routes we can take.
first, do we consider this potential for the repository to lock up
(without losing data) a show stopper?
if yes, then we need to pursue one of the ideas regarding sentinel
processes that mediate access to the db.
if not, then it seems prudent to make every effort to make it as
difficult as possible for the user to get into this situation. adding
cancelation support seems like a good step in this direction.
personally, i don't feel that it's a show stopper, as long as we try to
make it happen as little as possible, and provide good ways to clean up
(philip's proposed changes to svnadmin seem reasonable to me).
either way, i think adding cancelation support is a good idea, and the
question regarding that is: do we do it the way branko suggested (using
SVN_ERR), which i already have implemented, or do we want to have a
more invasive, but potentially cleaner for a multithreaded client,
solution like what bill proposes.
as one might expect, i vote for the patch i've got ;-)
i think it solves the problem we have now, it provides a means for
multithreaded clients to handle the issues associated with having
multiple cancelable svn functions happening at the same time, and it
doesn't have a huge effect on our codebase. i like the fact that this
is concealed behind SVN_ERR, and that we get cancelation practically
for free in almost all parts of the codebase.
thoughts?
-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
Re: Cancelling Subversion operations
Posted by Branko Čibej <br...@xbc.nu>.
Sander Striker wrote:
>>From: Garrett Rooney [mailto:rooneg@electricjellyfish.net]
>>Sent: 18 September 2002 22:07
>>
>>
>
>
>
>>Bill Tutt wrote:
>>
>>
>>
>>>Isn't this because we aren't talking to the datastore properly in the
>>>first place? Our database should NEVER get wedged. I thought we'd talked
>>>about how to fix this. We should work on that rather than this approach.
>>>If we're not willing to tackle that work item for 1.0 then I begin to
>>>wonder about what skewed ideas people might have about how
>>>stable/reliable a 1.0 relase of Subversion should be.
>>>
>>>
>>[snip]
>>
>> > If you could refresh my aging brain on why the database gets wedged and
>> > why we're not addressing that before doing things this way I'd greatly
>> > appreciate it.
>>
>>well, right now, it can easily get wedged just because of the fact that
>>there is no attempt to catch a SIGINT, so you hit control-c and your
>>process just exits. if you're holding locks inside berkeley db, then
>>you don't clean them up, so the repos can get wedged. without some kind
>>of cancelation support, this is unavoidable.
>>
>>
>
>Uhm. But we had quite some discussion on how to handle wedged repositories.
>Basically we need to fix the way we open the repository. Can we please
>revisit the proposed fixes in those threads first? That would effectively
>solve your problem aswell.
>
Can't be done, I tried. The only way to unwedge a repository is to run
recovery in a process that has exclusive access to the repo. If there
are wedged processes hung there, it won't work. The only way to avoid
hanging a process is to set a timeout on DB locks and transactions (can
be done in the DB_CONFIG file). Then you have to guess a correct timeout
period.
The automatic unwedging thing would work great if we had a DB server
process. But we don't.
--
Brane Čibej <br...@xbc.nu> http://www.xbc.nu/brane/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
RE: Cancelling Subversion operations
Posted by Sander Striker <st...@apache.org>.
> From: Garrett Rooney [mailto:rooneg@electricjellyfish.net]
> Sent: 18 September 2002 22:07
> Bill Tutt wrote:
>
> > Isn't this because we aren't talking to the datastore properly in the
> > first place? Our database should NEVER get wedged. I thought we'd talked
> > about how to fix this. We should work on that rather than this approach.
> > If we're not willing to tackle that work item for 1.0 then I begin to
> > wonder about what skewed ideas people might have about how
> > stable/reliable a 1.0 relase of Subversion should be.
>
> [snip]
>
> > If you could refresh my aging brain on why the database gets wedged and
> > why we're not addressing that before doing things this way I'd greatly
> > appreciate it.
>
> well, right now, it can easily get wedged just because of the fact that
> there is no attempt to catch a SIGINT, so you hit control-c and your
> process just exits. if you're holding locks inside berkeley db, then
> you don't clean them up, so the repos can get wedged. without some kind
> of cancelation support, this is unavoidable.
Uhm. But we had quite some discussion on how to handle wedged repositories.
Basically we need to fix the way we open the repository. Can we please
revisit the proposed fixes in those threads first? That would effectively
solve your problem aswell.
Sander
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Cancelling Subversion operations
Posted by Garrett Rooney <ro...@electricjellyfish.net>.
Bill Tutt wrote:
> Isn't this because we aren't talking to the datastore properly in the
> first place? Our database should NEVER get wedged. I thought we'd talked
> about how to fix this. We should work on that rather than this approach.
> If we're not willing to tackle that work item for 1.0 then I begin to
> wonder about what skewed ideas people might have about how
> stable/reliable a 1.0 relase of Subversion should be.
[snip]
> If you could refresh my aging brain on why the database gets wedged and
> why we're not addressing that before doing things this way I'd greatly
> appreciate it.
well, right now, it can easily get wedged just because of the fact that
there is no attempt to catch a SIGINT, so you hit control-c and your
process just exits. if you're holding locks inside berkeley db, then
you don't clean them up, so the repos can get wedged. without some kind
of cancelation support, this is unavoidable.
> If you'd be so kind as to reintegrate the cancellation editor then my
> objections about this approach would disappear completely. That way
> multi-threaded UIs can have what they want, and the command client can
> have what it wants.
ok, explain to me how you think the cancelation editor should work? the
client api doesn't have any means to pass in an editor anymore, that was
replaced with callback functions a little while ago, so it's not like
the client app can just wrap its own editor around whatever it's calling.
the way i see it, we still have to get the callback and the baton from
somewhere, and if we're going to have multiple batons, we have to pass
it explicitly to each function, otherwise i can't see how we're supposed
to get at it safely without locks. plus, we only get cancelation during
editor drives, which philip had some second thoughts about.
-garrett
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Cancelling Subversion operations
Posted by Bill Tutt <ra...@lyra.org>.
> From: Garrett Rooney [mailto:rooneg@electricjellyfish.net]
>
> Philip Martin wrote:
> > Garrett Rooney <ro...@electricjellyfish.net> writes:
> >
> >
> >>it means adding a couple
> >>of locks inside the callback, but how expensive is that really?
> >
> >
> > An uncontested mutex, while not free, can be cheap. The problem is
> > that it becomes expensive when contested, and it generally gets
worse
> > as the number of threads contesting it increases.
> >
> > Bill has raised a valid point, a single callback/baton is OK as a
> > simple async-signal handling mechanism, but as a general
cancellation
> > mechanism it's a poor solution.
>
> i agree it isn't a great solution, but it's a solution and it's here
> now. i started working on this for one reason: i want to be able to
> hit control-c when accessing a local repository without wedging the
> database.
Isn't this because we aren't talking to the datastore properly in the
first place? Our database should NEVER get wedged. I thought we'd talked
about how to fix this. We should work on that rather than this approach.
If we're not willing to tackle that work item for 1.0 then I begin to
wonder about what skewed ideas people might have about how
stable/reliable a 1.0 relase of Subversion should be.
> branko proposed a reasonably easy way to do this, so i took
> it on. to do so in a way that is the end-all be-all solution is a
much
> larger task, which involves touching quite a bit of code for a gain
that
> (at this point) is pretty theoretical.
>
If you could refresh my aging brain on why the database gets wedged and
why we're not addressing that before doing things this way I'd greatly
appreciate it.
> if someone else is willing to take on the task of doing this via the
way
> bill suggests (passing callbacks and batons down into each and ever
> function we want to allow cancelation in and inserting either
> cancelation editors or manual calling of the callback throughout the
> code), and is willing to commit to getting it done before 1.0, then
> fine, but if nobody's going to do that, i think this patch should go
in,
> since at least it solves the immediate problem, and it even provides
> (albeit not especially well) a way to solve more complex problems.
>
If you'd be so kind as to reintegrate the cancellation editor then my
objections about this approach would disappear completely. That way
multi-threaded UIs can have what they want, and the command client can
have what it wants.
Bill
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Cancelling Subversion operations
Posted by Garrett Rooney <ro...@electricjellyfish.net>.
Philip Martin wrote:
> Garrett Rooney <ro...@electricjellyfish.net> writes:
>
>
>>it means adding a couple
>>of locks inside the callback, but how expensive is that really?
>
>
> An uncontested mutex, while not free, can be cheap. The problem is
> that it becomes expensive when contested, and it generally gets worse
> as the number of threads contesting it increases.
>
> Bill has raised a valid point, a single callback/baton is OK as a
> simple async-signal handling mechanism, but as a general cancellation
> mechanism it's a poor solution.
i agree it isn't a great solution, but it's a solution and it's here
now. i started working on this for one reason: i want to be able to
hit control-c when accessing a local repository without wedging the
database. branko proposed a reasonably easy way to do this, so i took
it on. to do so in a way that is the end-all be-all solution is a much
larger task, which involves touching quite a bit of code for a gain that
(at this point) is pretty theoretical.
if someone else is willing to take on the task of doing this via the way
bill suggests (passing callbacks and batons down into each and ever
function we want to allow cancelation in and inserting either
cancelation editors or manual calling of the callback throughout the
code), and is willing to commit to getting it done before 1.0, then
fine, but if nobody's going to do that, i think this patch should go in,
since at least it solves the immediate problem, and it even provides
(albeit not especially well) a way to solve more complex problems.
-garrett
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
RE: RE: Cancelling Subversion operations
Posted by Jon Watte <sv...@mindcontrol.org>.
> > The logical extension if you wanted more expressive power is for SVN
> > to use thread-local storage to store the user context, but that seems
> > rather more effort than it's worth.
> If memory serves, thread-local storage isn't nearly as efficient as you
> might think on all OSs. It certainly is very efficient on Windows.
The three implementations I know of are:
Mask the stack pointer to get at it -- used by Windows I think.
Look in an otherwise unused CPU debug register to get at it --
used by BeOS I think.
Look it up in a hash table from thread ID -- used on MacOS I
think.
If someone good at Linux, Solaris and BSD internals could fill
in the details for those OS-es, I'd appreciate it (just for my
personal edification if nothing else).
Anyway, if testing for cancellation involves looking up in a
hash table every time you do it, that might possibly actually
show up in a profiler (even when compared to, say, network or
disk I/O...)
I guess the main point is that with the current suggested
implementation, there actually is sufficient expressive power
to make a threaded GUI implementation only cancel one of several
outstanding requests, even if there is only one global baton.
And only that threaded GUI implementation actually needs to worry
about whether TLS is efficient or not.
Cheers,
/ h+
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
RE: RE: Cancelling Subversion operations
Posted by Bill Tutt <ra...@lyra.org>.
> From: Jon Watte [mailto:svn-dev-0209@mindcontrol.org]
>
>
> > Bill has raised a valid point, a single callback/baton is OK as a
> > simple async-signal handling mechanism, but as a general
cancellation
> > mechanism it's a poor solution.
>
> It seems that any threaded implementation can just use thread-local
> storage of one for or another for its per-running-operation needs.
> Thus I think the interface as specified is sufficiently powerful.
> This is assuming that the same THREAD will only run one SVN operation
> at a time, which seems reasonable.
>
> The logical extension if you wanted more expressive power is for SVN
> to use thread-local storage to store the user context, but that seems
> rather more effort than it's worth.
>
If memory serves, thread-local storage isn't nearly as efficient as you
might think on all OSs. It certainly is very efficient on Windows.
Bill
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
RE: Cancelling Subversion operations
Posted by Jon Watte <sv...@mindcontrol.org>.
> Bill has raised a valid point, a single callback/baton is OK as a
> simple async-signal handling mechanism, but as a general cancellation
> mechanism it's a poor solution.
It seems that any threaded implementation can just use thread-local
storage of one for or another for its per-running-operation needs.
Thus I think the interface as specified is sufficiently powerful.
This is assuming that the same THREAD will only run one SVN operation
at a time, which seems reasonable.
The logical extension if you wanted more expressive power is for SVN
to use thread-local storage to store the user context, but that seems
rather more effort than it's worth.
Cheers,
/ h+
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Cancelling Subversion operations
Posted by Philip Martin <ph...@codematters.co.uk>.
Garrett Rooney <ro...@electricjellyfish.net> writes:
> it means adding a couple
> of locks inside the callback, but how expensive is that really?
An uncontested mutex, while not free, can be cheap. The problem is
that it becomes expensive when contested, and it generally gets worse
as the number of threads contesting it increases.
Bill has raised a valid point, a single callback/baton is OK as a
simple async-signal handling mechanism, but as a general cancellation
mechanism it's a poor solution.
I'm not sure I agree with his other comment about some operations not
needing to be cancellable (post-commit processing and downloading a
large file were the examples). A process that ignores SIGTERM, or
just delays too long before handling it, is likely to get SIGKILL.
--
Philip Martin
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Cancelling Subversion operations
Posted by Garrett Rooney <ro...@electricjellyfish.net>.
Bill Tutt wrote:
> No it doesn't. If the last patch I submitted it didn't contain it, it's
> clearly quite easy to just pass in the cancellation callback, and baton
> to every appropriate entry point that needs to construct and compose the
> cancellation editor. (Assuming you want to use the cancellation editor.)
good point.
> I don't care so much about using the cancellation editor approach. I
> care more about providing the ability to specify more than one
> cancellation baton. Being able to specify more than one callback
> function would be nice too, but applications can work around that more
> easily than by not being able to specify more than one cancellation
> baton.
unfortunately, having multiple cancelation batons means that we can't
just stick this check inside SVN_ERR, we'd actually have to insert calls
to the cancelation callback all over the code. in many cases a
cancelation editor could take care of this, but probably not all of them.
do we want to go down this road? i mean how much worse off is it for
the client applications if they have to work with a single baton? how
common is the 'i have multiple different threads all doing subversion
stuff at the same time' case, and will working with a single
callback/baton be a huge penalty for them? it means adding a couple of
locks inside the callback, but how expensive is that really? i'd
personally be willing to go with the current implementation we've got
until someone can show us a concrete example of it being a horrible
speed problem.
-garrett
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
RE: Re: Cancelling Subversion operations
Posted by Bill Tutt <ra...@lyra.org>.
> From: Garrett Rooney [mailto:rooneg@electricjellyfish.net]
>
> Philip Martin wrote:
> > "Bill Tutt" <ra...@lyra.org> writes:
> >
> >
> >>>Which cancellation editor? Is there some mechanism already in
place?
> >>>Grepping the source code for "cancel" doesn't reveal anything.
> >>
> >>Once upon a time, (dunno if the code is still there), I had
submitted a
> >>patch for a cancellation editor that got composed with the update
and
> >>commit editors. This allowed per operation cancellation baton state
to
> >>be used and leveraged.
> >
> >
> > I don't see anything obvious in the current code. I have found some
> > patches in the mailing list from about nine months ago, a combined
> > trace/cancel set, I guess they never made it into the code base.
>
> it was committed at one point, but was removed some time after, i'm
not
> sure why.
>
> > Well, the approach in Garrett's patch does allow an application to
> > cancel on the basis of thread identity, and if the application
tracks
> > which operation each thread is running it can decide which threads
to
> > cancel.
> >
> > The cancellation editor approach fits in neatly with the other
editor
> > code. I wonder how much bit-rot your patches have suffered over the
> > months? I think the trace part is redundant, but the cancel part
may
> > be worth investigating.
>
> the problem with the cancelation editor (in my mind) is that it
requires
> the client app to know that we're using editors, and we've been moving
> away from that (that's why we know longer use trace editors, now we
have
> callback functions). the fact that we use editors internally is an
> implementation detail, and is not something that the client
application
> should know about or care about.
>
No it doesn't. If the last patch I submitted it didn't contain it, it's
clearly quite easy to just pass in the cancellation callback, and baton
to every appropriate entry point that needs to construct and compose the
cancellation editor. (Assuming you want to use the cancellation editor.)
I don't care so much about using the cancellation editor approach. I
care more about providing the ability to specify more than one
cancellation baton. Being able to specify more than one callback
function would be nice too, but applications can work around that more
easily than by not being able to specify more than one cancellation
baton.
Bill
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Cancelling Subversion operations
Posted by Garrett Rooney <ro...@electricjellyfish.net>.
Philip Martin wrote:
> "Bill Tutt" <ra...@lyra.org> writes:
>
>
>>>Which cancellation editor? Is there some mechanism already in place?
>>>Grepping the source code for "cancel" doesn't reveal anything.
>>
>>Once upon a time, (dunno if the code is still there), I had submitted a
>>patch for a cancellation editor that got composed with the update and
>>commit editors. This allowed per operation cancellation baton state to
>>be used and leveraged.
>
>
> I don't see anything obvious in the current code. I have found some
> patches in the mailing list from about nine months ago, a combined
> trace/cancel set, I guess they never made it into the code base.
it was committed at one point, but was removed some time after, i'm not
sure why.
> Well, the approach in Garrett's patch does allow an application to
> cancel on the basis of thread identity, and if the application tracks
> which operation each thread is running it can decide which threads to
> cancel.
>
> The cancellation editor approach fits in neatly with the other editor
> code. I wonder how much bit-rot your patches have suffered over the
> months? I think the trace part is redundant, but the cancel part may
> be worth investigating.
the problem with the cancelation editor (in my mind) is that it requires
the client app to know that we're using editors, and we've been moving
away from that (that's why we know longer use trace editors, now we have
callback functions). the fact that we use editors internally is an
implementation detail, and is not something that the client application
should know about or care about.
-garrett
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
RE: Re: Cancelling Subversion operations
Posted by Bill Tutt <ra...@lyra.org>.
> From: Philip Martin [mailto:philip@codematters.co.uk]
>
> "Bill Tutt" <ra...@lyra.org> writes:
>
> > > Which cancellation editor? Is there some mechanism already in
place?
> > > Grepping the source code for "cancel" doesn't reveal anything.
> >
> > Once upon a time, (dunno if the code is still there), I had
submitted a
> > patch for a cancellation editor that got composed with the update
and
> > commit editors. This allowed per operation cancellation baton state
to
> > be used and leveraged.
>
> I don't see anything obvious in the current code. I have found some
> patches in the mailing list from about nine months ago, a combined
> trace/cancel set, I guess they never made it into the code base.
>
That wouldn't surprise me. :(
> > > > Doing it this way lets the user application maintain its state
> however
> > > > it wants, and not in some bizarre way that's necessitated by
> > > > Subversion's API choice.
> > > >
> > > > It just so happens that the command line client doesn't care so
much,
> > > > but other more sophisticated users of the API might.
> > >
> > > It also happens to be fairly simple to implement. I'm not sure
how a
> > > general cancellation mechanism would work, but I suspect it may
> > > involve passing some sort of context baton through every
subversion
> > > function. That may not be a bad thing to do anyway; I'm wondering
it
> > > it would be a better approach to the access baton stuff.
>
> From a quick browse through the patches it seems that it would allow
> the application to cancel at each editor function. That does not
> introduce as many cancellation points as Garrett's patch, but it may
> be enough.
The cancellation editor idea was to try and provide a tradeoff between
always calling the cancellation callback everywhere, and trying to
ensure that the cancellation callback got called before every possibly
expensive network touching operation. (i.e. mostly trying to ensure that
a multi-threaded UI could have a chance in cancellation the current
operation in progress)
> A few places would not allow cancellation, post-commit
> processing for example, I don't know if that's a problem. I suppose
> it depends on exactly how many operations fit into the cancellation
> editor scheme: what about transmitting postfix text deltas during a
> commit, or receiving a large file during an update?
>
If you're canceling a commit, then we don't care where the cancellation
happens. Post-commit processing should be asynchronous anyway. The user
application shouldn't be able to cancel post-commit processing except
from within the post-commit processing code.
> > > What sort of thing do you see a sophisticated API user doing when
it
> > > repsonds to an asynchronous signal?
> >
> > The user API might have multiple Subversion operation contexts
because
> > the GUI is multi-threaded. One thread for user directed requests,
and
> > another for background communication requests to lazily update the
UI
> > every so often.
> >
> > The UI might very well want to have different cancellation baton
data
> > for each UI context.
>
> Well, the approach in Garrett's patch does allow an application to
> cancel on the basis of thread identity, and if the application tracks
> which operation each thread is running it can decide which threads to
> cancel.
>
At the cost of a bunch of synchronizing objects for each call into the
cancellation object. Ick...
> The cancellation editor approach fits in neatly with the other editor
> code. I wonder how much bit-rot your patches have suffered over the
> months? I think the trace part is redundant, but the cancel part may
> be worth investigating.
Got me. I don't really care about the cancellation editor continuing to
exist as I wrote it originally. I just want Subversion's public API to
support the ability of the user application to supply multiple
cancellation batons.
If you use an updated version of the patch, great, if not I don't care.
Just give our API users the ability to pass in multiple cancellation
batons.
Bill
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Cancelling Subversion operations
Posted by Philip Martin <ph...@codematters.co.uk>.
"Bill Tutt" <ra...@lyra.org> writes:
> > Which cancellation editor? Is there some mechanism already in place?
> > Grepping the source code for "cancel" doesn't reveal anything.
>
> Once upon a time, (dunno if the code is still there), I had submitted a
> patch for a cancellation editor that got composed with the update and
> commit editors. This allowed per operation cancellation baton state to
> be used and leveraged.
I don't see anything obvious in the current code. I have found some
patches in the mailing list from about nine months ago, a combined
trace/cancel set, I guess they never made it into the code base.
> > > Doing it this way lets the user application maintain its state however
> > > it wants, and not in some bizarre way that's necessitated by
> > > Subversion's API choice.
> > >
> > > It just so happens that the command line client doesn't care so much,
> > > but other more sophisticated users of the API might.
> >
> > It also happens to be fairly simple to implement. I'm not sure how a
> > general cancellation mechanism would work, but I suspect it may
> > involve passing some sort of context baton through every subversion
> > function. That may not be a bad thing to do anyway; I'm wondering it
> > it would be a better approach to the access baton stuff.
Re: Cancelling Subversion operations
Posted by Garrett Rooney <ro...@electricjellyfish.net>.
Bill Tutt wrote:
>
>>From: Philip Martin [mailto:philip@codematters.co.uk]
>>
>>"Bill Tutt" <ra...@lyra.org> writes:
>>
>>
>>>Oh yeah, and what are you thinking having only one user cancellation
>>>handler?
>>>i.e. Why is this state global?
>>
>>The simple answer is that it handles a global condition, namely an
>>asynchronous signal.
>
>
> No, it doesn't. The user of it might only need a global condition, but
> it's incorrect of an API to assume that it's really a global condition.
>
> The command line client's use of the API is a global use of course, esp.
> since logic gets invoked via a signal handler, but that doesn't mean
> that another user application would do the same thing.
FWIW, the fact that a GUI client might have more complex needs than the
command line client is the reason the proposed solution uses a client
specified callback. the previous proposals had just a simple volatile
sig_atomic_t with a 'svn_async_cancel' function that would twiddle it.
then SVN_ERR would check that variable. this would ONLY be useful for a
signal handler type situation (or a single threaded client app), which
was why i pushed for the callback. this way if the client has more
complex needs, they can provide a callback function which takes that
into account. they still have to go through a single callback function,
but it is certainly possible to write that function to handle a complex
multithreaded app.
-garrett
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
RE: Re: Cancelling Subversion operations
Posted by Bill Tutt <ra...@lyra.org>.
> From: Philip Martin [mailto:philip@codematters.co.uk]
>
> "Bill Tutt" <ra...@lyra.org> writes:
>
> > Oh yeah, and what are you thinking having only one user cancellation
> > handler?
> > i.e. Why is this state global?
>
> The simple answer is that it handles a global condition, namely an
> asynchronous signal.
No, it doesn't. The user of it might only need a global condition, but
it's incorrect of an API to assume that it's really a global condition.
The command line client's use of the API is a global use of course, esp.
since logic gets invoked via a signal handler, but that doesn't mean
that another user application would do the same thing.
> > There was a good reason that the cancellation editor was written,
it's
> > so that the cancellation handler could be specified for each
particular
> > and interesting operation.
>
> Which cancellation editor? Is there some mechanism already in place?
> Grepping the source code for "cancel" doesn't reveal anything.
>
Once upon a time, (dunno if the code is still there), I had submitted a
patch for a cancellation editor that got composed with the update and
commit editors. This allowed per operation cancellation baton state to
be used and leveraged.
> > Doing it this way lets the user application maintain its state
however
> > it wants, and not in some bizarre way that's necessitated by
> > Subversion's API choice.
> >
> > It just so happens that the command line client doesn't care so
much,
> > but other more sophisticated users of the API might.
>
> It also happens to be fairly simple to implement. I'm not sure how a
> general cancellation mechanism would work, but I suspect it may
> involve passing some sort of context baton through every subversion
> function. That may not be a bad thing to do anyway; I'm wondering it
> it would be a better approach to the access baton stuff.
>
> What sort of thing do you see a sophisticated API user doing when it
> repsonds to an asynchronous signal?
>
The user API might have multiple Subversion operation contexts because
the GUI is multi-threaded. One thread for user directed requests, and
another for background communication requests to lazily update the UI
every so often.
The UI might very well want to have different cancellation baton data
for each UI context.
FYI,
Bill
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Cancelling Subversion operations
Posted by Philip Martin <ph...@codematters.co.uk>.
"Bill Tutt" <ra...@lyra.org> writes:
> Oh yeah, and what are you thinking having only one user cancellation
> handler?
> i.e. Why is this state global?
The simple answer is that it handles a global condition, namely an
asynchronous signal.
> There was a good reason that the cancellation editor was written, it's
> so that the cancellation handler could be specified for each particular
> and interesting operation.
Which cancellation editor? Is there some mechanism already in place?
Grepping the source code for "cancel" doesn't reveal anything.
> Doing it this way lets the user application maintain its state however
> it wants, and not in some bizarre way that's necessitated by
> Subversion's API choice.
>
> It just so happens that the command line client doesn't care so much,
> but other more sophisticated users of the API might.
It also happens to be fairly simple to implement. I'm not sure how a
general cancellation mechanism would work, but I suspect it may
involve passing some sort of context baton through every subversion
function. That may not be a bad thing to do anyway; I'm wondering it
it would be a better approach to the access baton stuff.
What sort of thing do you see a sophisticated API user doing when it
repsonds to an asynchronous signal?
--
Philip Martin
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Cancelling Subversion operations
Posted by Nuutti Kotivuori <na...@iki.fi>.
Bill Tutt wrote:
> Oh yeah, and what are you thinking having only one user cancellation
> handler?
> i.e. Why is this state global?
>
> There was a good reason that the cancellation editor was written,
> it's so that the cancellation handler could be specified for each
> particular and interesting operation.
>
> Doing it this way lets the user application maintain its state
> however it wants, and not in some bizarre way that's necessitated by
> Subversion's API choice.
>
> It just so happens that the command line client doesn't care so
> much, but other more sophisticated users of the API might.
This issue was raised a bit earlier in the discussion. Basically it
seemed like the only real "answer" to that was that "It's simpler this
way". Or then I missed something.
I would think this decision would need some written rationale
somewhere, because it's bound to creep up on some unsuspecting client
coder and initiate the "BOO!" "Eek!" communication model.
-- Naked
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Cancelling Subversion operations
Posted by Bill Tutt <ra...@lyra.org>.
> From: Garrett Rooney [mailto:rooneg@electricjellyfish.net]
>
> On Tuesday, September 17, 2002, at 07:39 PM, Philip Martin wrote:
>
> > Garrett Rooney <ro...@electricjellyfish.net> writes:
> >
> >> the last remaining question is "if signal itself isn't portable, is
> >> sig_atomic_t?" according to all i've read, and what philip has
said,
> >> about the only thing that's safe to do in a signal handler is to
set a
> >> sig_atomic_t variable... if that typedef isn't considered
portable,
> >> it complicates life considerably...
> >
> > apr_atomic_t? It appears to be unsigned int on my platform, whereas
> > sig_atomic_t is plain int. That probably doesn't matter, and if it
> > does it's APR's problem!
>
> the thing that worries me about apr_atomic_t is that if atomic ops
> aren't available on the platform, it falls back to an implementation
> using mutexes, which means that apr_atomic_{inc,dec,get,set} wouldn't
> be safe to use in a signal handler.
>
You know if synchronization structures aren't safe to use inside a
signal handler, then your platform is in pretty sad shape.
If apr_atomic_t isn't good enough, then you might as well just mark the
state variable as volatile. You still won't get perfect SMP cache
coherency, but at least you'll be slightly better off. My take on APR's
apr_atomic_t is that some preprocessing symbol should be defined if
apr_atomic_t operations aren't simple assembly instructions ala "lock
xchg" on x86, etc.
Oh yeah, and what are you thinking having only one user cancellation
handler?
i.e. Why is this state global?
There was a good reason that the cancellation editor was written, it's
so that the cancellation handler could be specified for each particular
and interesting operation.
Doing it this way lets the user application maintain its state however
it wants, and not in some bizarre way that's necessitated by
Subversion's API choice.
It just so happens that the command line client doesn't care so much,
but other more sophisticated users of the API might.
Bill
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: permissions (and other) problems
Posted by Garrett Rooney <ro...@electricjellyfish.net>.
On Tuesday, September 17, 2002, at 07:39 PM, Philip Martin wrote:
> Garrett Rooney <ro...@electricjellyfish.net> writes:
>
>> the last remaining question is "if signal itself isn't portable, is
>> sig_atomic_t?" according to all i've read, and what philip has said,
>> about the only thing that's safe to do in a signal handler is to set a
>> sig_atomic_t variable... if that typedef isn't considered portable,
>> it complicates life considerably...
>
> apr_atomic_t? It appears to be unsigned int on my platform, whereas
> sig_atomic_t is plain int. That probably doesn't matter, and if it
> does it's APR's problem!
the thing that worries me about apr_atomic_t is that if atomic ops
aren't available on the platform, it falls back to an implementation
using mutexes, which means that apr_atomic_{inc,dec,get,set} wouldn't
be safe to use in a signal handler.
-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
Re: permissions (and other) problems
Posted by Philip Martin <ph...@codematters.co.uk>.
Garrett Rooney <ro...@electricjellyfish.net> writes:
> the last remaining question is "if signal itself isn't portable, is
> sig_atomic_t?" according to all i've read, and what philip has said,
> about the only thing that's safe to do in a signal handler is to set a
> sig_atomic_t variable... if that typedef isn't considered portable,
> it complicates life considerably...
apr_atomic_t? It appears to be unsigned int on my platform, whereas
sig_atomic_t is plain int. That probably doesn't matter, and if it
does it's APR's problem!
--
Philip Martin
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: permissions (and other) problems
Posted by Garrett Rooney <ro...@electricjellyfish.net>.
On Tuesday, September 17, 2002, at 06:23 PM, Barry Scott wrote:
> Posix standardised signals not ANSI C. Posix uses
> sigaction() and friends.
>
> There is also the pre-posix signals on old Unix API,
> signal() etc.
>
> Atleast on HP-UX there are both pre-posix and posix signals.
> If you mix them your code breaks is subtle ways.
>
> APR is essential to hide this mess and allow portability
> across Unix let along the none Unix OS.
ok, after looking at the code for apr_signal, i can see why we need
something to mediate all the random weird-ass implementations out
there, but from what i've been able to find (on the net and in K&R and
harbison and steele's 'C, a reference manual'), it seems that
rudimentary signals are part of ANSI C. that said, i am using
apr_signal, because there is still a significant portability problem.
the last remaining question is "if signal itself isn't portable, is
sig_atomic_t?" according to all i've read, and what philip has said,
about the only thing that's safe to do in a signal handler is to set a
sig_atomic_t variable... if that typedef isn't considered portable, it
complicates life considerably...
-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
RE: permissions (and other) problems
Posted by Barry Scott <ba...@ntlworld.com>.
Posix standardised signals not ANSI C. Posix uses
sigaction() and friends.
There is also the pre-posix signals on old Unix API,
signal() etc.
Atleast on HP-UX there are both pre-posix and posix signals.
If you mix them your code breaks is subtle ways.
APR is essential to hide this mess and allow portability
across Unix let along the none Unix OS.
BArry
> i'm not sure. signal is theoretically part of the c standard, but for
> consistency's sake, we should probably use the one from apr. i'll look
> into it tonight and figure out what to do before committing.
>
> thanks,
>
> -garrett
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: permissions (and other) problems
Posted by Garrett Rooney <ro...@electricjellyfish.net>.
Philip Martin wrote:
> Garrett Rooney <ro...@electricjellyfish.net> writes:
>
>
>>Index: subversion/clients/cmdline/main.c
>>===================================================================
>>--- subversion/clients/cmdline/main.c
>>+++ subversion/clients/cmdline/main.c Mon Sep 16 21:46:55 2002
>
> [snip]
>
>>@@ -913,6 +926,10 @@
>>
>> /* Create our top-level pool. */
>> pool = svn_pool_create (NULL);
>>+
>>+ svn_cancel_set_cancel_func (check_cancelled, NULL, svn_pool_create (pool));
>>+
>>+ signal (SIGINT, cancel);
>
>
> APR provides apr_signal in apr_signal.h, should we use that?
i'm not sure. signal is theoretically part of the c standard, but for
consistency's sake, we should probably use the one from apr. i'll look
into it tonight and figure out what to do before committing.
thanks,
-garrett
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: permissions (and other) problems
Posted by Philip Martin <ph...@codematters.co.uk>.
Garrett Rooney <ro...@electricjellyfish.net> writes:
> Index: subversion/clients/cmdline/main.c
> ===================================================================
> --- subversion/clients/cmdline/main.c
> +++ subversion/clients/cmdline/main.c Mon Sep 16 21:46:55 2002
[snip]
> @@ -913,6 +926,10 @@
>
> /* Create our top-level pool. */
> pool = svn_pool_create (NULL);
> +
> + svn_cancel_set_cancel_func (check_cancelled, NULL, svn_pool_create (pool));
> +
> + signal (SIGINT, cancel);
APR provides apr_signal in apr_signal.h, should we use that?
>
> /* Begin processing arguments. */
> memset (&opt_state, 0, sizeof (opt_state));
--
Philip Martin
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: permissions (and other) problems
Posted by Garrett Rooney <ro...@electricjellyfish.net>.
On Sunday, September 15, 2002, at 10:51 PM, Garrett Rooney wrote:
> here's what i've currently got. the patch has two known issues.
> first, for some reason if i use a subpool of the main pool as the
> cancelation pool, we segfault trying to allocate the cancelation
> error. i have no clue why this is happening. if someone could shed
> some light on the problem, i would appreciate it.
just figured this one out. when the top level pool is destroyed, all
the cleanup functions get called, some of which use SVN_ERR. since
we're in a state where the cancelation function is returning true, this
causes a SVN_ERR_CANCELED error to be allocated. if the cancelation
pool is not the last to go (which it isn't), then we crash.
the current patch i've got sets up a cleanup function that unregisters
the cancelation function when the cancelation pool is destroyed (which
makes sense, since the cancelation function is pretty useless without a
pool to allocate errors from). i'm not sure about the thread safety
aspects of this though. my current thoughts are that any time that
pool is destroyed while another thread is using svn functions it's an
application bug, so we shouldn't go out of our way to make it work.
thoughts?
> second, there's one place (that i noticed) where svn_error_clear_all
> is used in a function which does not return an svn_error_t *, so we
> seem to be stuck not having cancelation supported from there. i guess
> as long as this doesn't happen often, it's ok, but i still don't like
> it.
i've concluded that this is just something we have to live with.
other than that, i'm about ready to commit. it'd be nice to rework the
svn_error_clear_all function to incorporate function and line numbers
like the other error code does, but that can come in a second commit
down the line.
here's the current patch i'm working on, as always i'd appreciate any
comments.
-garrett
Re: permissions (and other) problems
Posted by Garrett Rooney <ro...@electricjellyfish.net>.
On Saturday, September 14, 2002, at 03:39 PM, Philip Martin wrote:
> Garrett Rooney <ro...@electricjellyfish.net> writes:
>
>> i thought of that, but it raises an interesting question. is it more
>> important to get the line number of where you were canceled, or to
>> have the user defined cancelation function be able to annotate the
>> error with some indication of why you were canceled? something worth
>> thinking about.
>
> Well, the client can record and report the reason independently of the
> svn_error_t. The line number information, on the other hand, once
> lost has gone for ever.
yeah, that makes sense. my current patch (attatched to this email)
goes with svn_boolean_t as the return type.
>>> Note that as far as asynchronous signals go, it doesn't really matter
>>> whether SVN_ERR calls the cancel function before or after expr, so it
>>> can be written
>>
>> i thought of that, but i figured since we've already been told to stop
>> doing work, we might as well make that check happen before we go off
>> and do any other work.
>
> You have missed that the cancel state will (usually) get set
> asynchronously. So it may not be set before calling expr, it may get
> set *during* expr, and then be set after expr. In general, it makes
> no difference whether you check before or after expr, neither one is
> more immediate, or "faster", than the other. So just pick which ever
> produces the cleanest code.
as usual, you make a lot of sense ;-)
here's what i've currently got. the patch has two known issues.
first, for some reason if i use a subpool of the main pool as the
cancelation pool, we segfault trying to allocate the cancelation error.
i have no clue why this is happening. if someone could shed some
light on the problem, i would appreciate it. second, there's one place
(that i noticed) where svn_error_clear_all is used in a function which
does not return an svn_error_t *, so we seem to be stuck not having
cancelation supported from there. i guess as long as this doesn't
happen often, it's ok, but i still don't like it.
another thing that's popped up is that if i control-c out of a long
running update, we end up leaving lockfiles all over the place, even
though we errored out with SVN_ERR_CANCELED. if someone who has more
clue about how the locking in libsvn_wc works could explain how this
can happen, i'd appreciate it. should there be an error condition
where we leave the working copy locked? would it make sense to put
checks for SVN_ERR_CANCELED in places where this kind of things could
happen?
-garrett
Re: permissions (and other) problems
Posted by Philip Martin <ph...@codematters.co.uk>.
Garrett Rooney <ro...@electricjellyfish.net> writes:
> i thought of that, but it raises an interesting question. is it more
> important to get the line number of where you were canceled, or to
> have the user defined cancelation function be able to annotate the
> error with some indication of why you were canceled? something worth
> thinking about.
Well, the client can record and report the reason independently of the
svn_error_t. The line number information, on the other hand, once
lost has gone for ever.
> > Note that as far as asynchronous signals go, it doesn't really matter
> > whether SVN_ERR calls the cancel function before or after expr, so it
> > can be written
>
> i thought of that, but i figured since we've already been told to stop
> doing work, we might as well make that check happen before we go off
> and do any other work.
You have missed that the cancel state will (usually) get set
asynchronously. So it may not be set before calling expr, it may get
set *during* expr, and then be set after expr. In general, it makes
no difference whether you check before or after expr, neither one is
more immediate, or "faster", than the other. So just pick which ever
produces the cleanest code.
--
Philip Martin
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: permissions (and other) problems
Posted by Garrett Rooney <ro...@electricjellyfish.net>.
On Saturday, September 14, 2002, at 01:58 PM, Philip Martin wrote:
>
> That means that the error's debugging line number won't be useful. It
> may be better for the cancel function to return boolean and for
> SVN_ERR to generate the error. Just have the client specify a pool
> when it specifies the cancel function.
i thought of that, but it raises an interesting question. is it more
important to get the line number of where you were canceled, or to have
the user defined cancelation function be able to annotate the error
with some indication of why you were canceled? something worth
thinking about.
> Note that as far as asynchronous signals go, it doesn't really matter
> whether SVN_ERR calls the cancel function before or after expr, so it
> can be written
>
> svn_error_t *svn_err__temp = (expr);
> if (svn_cancel__func
> && svn_cancel__func (svn_cancel__baton)
> && ! (svn_err__temp && svn_err__temp->apr_err == ERR_CANCEL))
> svn_err__temp
> = svn_error_create (ERR_CANCEL, 0, svn_err__temp,
> svn_cancel__pool,
> "Drat! Foiled again!");
> if (svn_err__temp)
> return svn_err__temp;
i thought of that, but i figured since we've already been told to stop
doing work, we might as well make that check happen before we go off
and do any other work. it's probably not important one way or the
other though, and yours does seem simpler.
>> 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,
>
> Not necessary, they only get used synchronously. The function may
> access a volatile flag when it runs, but Subversion doesn't see that.
good point.
>> 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).
>
> Yup.
ok, cool. i'll work up a patch using this technique later today.
-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
Re: permissions (and other) problems
Posted by Philip Martin <ph...@codematters.co.uk>.
Garrett Rooney <ro...@isris.pair.com> writes:
> 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.
That means that the error's debugging line number won't be useful. It
may be better for the cancel function to return boolean and for
SVN_ERR to generate the error. Just have the client specify a pool
when it specifies the cancel function.
typedef svn_boolean_t (*svn_cancel_func_t) (void *cancel_baton);
void svn_set_cancel_func (svn_cancel_func_t cancel_func,
void *cancel_baton,
apr_pool_t *cancel_pool);
Note that as far as asynchronous signals go, it doesn't really matter
whether SVN_ERR calls the cancel function before or after expr, so it
can be written
svn_error_t *svn_err__temp = (expr);
if (svn_cancel__func
&& svn_cancel__func (svn_cancel__baton)
&& ! (svn_err__temp && svn_err__temp->apr_err == ERR_CANCEL))
svn_err__temp
= svn_error_create (ERR_CANCEL, 0, svn_err__temp, svn_cancel__pool,
"Drat! Foiled again!");
if (svn_err__temp)
return svn_err__temp;
>
> 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,
Not necessary, they only get used synchronously. The function may
access a volatile flag when it runs, but Subversion doesn't see that.
> 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).
Yup.
--
Philip Martin
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: permissions (and other) problems
Posted by Philip Martin <ph...@codematters.co.uk>.
Garrett Rooney <ro...@isris.pair.com> writes:
> On Sat, Sep 14, 2002 at 10:39:31AM +0100, Philip Martin wrote:
> > Branko Äibej <br...@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 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.
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!
--
Philip Martin
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: permissions (and other) problems
Posted by Philip Martin <ph...@codematters.co.uk>.
Branko Äibej <br...@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.
>
>
> 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.
> 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.
> 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.
--
Philip Martin
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: permissions (and other) problems
Posted by mark benedetto king <bk...@inquira.com>.
On Sat, Sep 14, 2002 at 02:36:47AM +0200, Branko ?ibej wrote:
>
> You only need the one state. Here's the pseudocode:
For pseudocode, that looks suspiciously like python. :-)
--ben
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: permissions (and other) problems
Posted by Branko Čibej <br...@xbc.nu>.
Garrett Rooney wrote:
> there are actually two states we need to worry about. the first is
> 'have we been canceled', which tells SVN_ERR and SVN_ERR_W to return
> an SVN_ERR_CANCELED error, and the second is 'are we in the process of
> backing out because we received a cancelation', which causes
> svn_error_clear_all to return an SVN_ERR_CANCELED error.
You only need the one state. Here's the pseudocode:
# global state
apr_atomic_t svn_cancelled = false
apr_mutex_t svn_cancellation_mutex
apr_pool_t *svn_cancellation_pool = null
# public interfaces
def svn_async_cancel(pool):
svn_cancellation_mutex.lock()
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()
def svn_async_cancel_clear():
svn_cancellation_mutex.lock()
apr_atoomic_set(svn_canceled, false)
svn_cancellation_pool = null
svn_cancellation_mutex.unlock()
def SVN_ERR(err):
if apr_atomic_get(svn_canceled):
return svn_error_cancel(err)
if err is not null:
return err
def SVN_ERR_W(err, msg):
if apr_atomic_get(svn_canceled):
return svn_error_cancel(err)
if err is not null:
return svn_error_quick_wrap(err, msg)
def svn_error_clear_all(err):
if apr_atomic_get(svn_canceled):
return svn_error_cancel(err)
# clean up errors, etc.
def svn_error_cancel(err):
if err is not null:
# Just propagate cancellation errors
if err->svn_err is SVN_ERR_CANCELLED:
return err
return svn_error_create(SVN_ERR_CANCELLED, err->pool)
svn_cancellation_mutex.lock()
if svn_cancellation_pool is null:
# Oops, a race condition? help! WE have to document that when
# you call svn_async_cancel, you should _not_ call svn_async_clear
# until you're absolutely sure your working threads have processed
# the cancellation. You can't cancel a cancel!
abort()
err = svn_error_create(SVN_ERR_CANCELLED, svn_cancellation_pool)
svn_cancellation_mutex.unlock()
return err
Regarding the implementation of svn_err_cancel and error locations: Look
at how the other error creation routines are implemented, and note that
svn_error.c undefines the macro wrappers first. With this
implementation, the SVN_ERR_CANCELLED errors will have correct location
info (i.e., they'll come from the SVN_ERR macros). It would make sense
to create a location wrapper for svn_error_clear_all, too; and note that
you should pass the incoming error to svn_error_create_all, so that we
don't get duplicate SVN_ERR_CANCELLED errors.
--
Brane Čibej <br...@xbc.nu> http://www.xbc.nu/brane/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: permissions (and other) problems
Posted by Garrett Rooney <ro...@electricjellyfish.net>.
Garrett Rooney wrote:
>>> +static volatile sig_atomic_t canceled = FALSE;
>>
>>
>>
>> Is canceled an Americanism? I'd have spelt it cancelled.
>
>
> it's more of a "garrett can't spell"ism ;-)
actually, i think we're both right.
www.webster.com seems to feel that both canceled and cancelled are correct.
-garrett
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: permissions (and other) problems
Posted by Garrett Rooney <ro...@electricjellyfish.net>.
Philip Martin wrote:
> Garrett Rooney <ro...@electricjellyfish.net> writes:
>
>
>>Index: subversion/include/svn_error.h
>>===================================================================
>>--- subversion/include/svn_error.h
>>+++ subversion/include/svn_error.h Thu Sep 12 22:49:39 2002
>
> [snip]
>
>>@@ -151,6 +175,8 @@
>> svn_error_t *svn_err__temp = (expr); \
>> if (svn_err__temp) \
>> return svn_err__temp; \
>>+ else if (svn__async_canceled ()) \
>>+ return svn__async_cancelation_error (); \
>
>
> Generating the error by calling a function will mean that the
> debugging line number associated with the error will refer to that
> function. If you called svn_error_create directly would mean that the
> line number would refer to the SVN_ERR macro, which is much more
> interesting, although it may increase the code size.
The reason I moved it into the function was that i needed to get a pool
from somewhere to do the allocation. i suppose i could define a
get_cancelation_pool function, and move the error creation into the ERR
macros... then you could see where exactly you were when you got
canceled, which would be nice.
>> } while (0)
>>
>>
>>Index: subversion/libsvn_subr/cancel.c
>>===================================================================
>>--- subversion/libsvn_subr/cancel.c
>>+++ subversion/libsvn_subr/cancel.c Thu Sep 12 22:36:31 2002
>>@@ -0,0 +1,80 @@
>>+/*
>>+ * cancel.c: support for asyncronous cancelation of subversion functions
>>+ *
>>+ * ====================================================================
>>+ * Copyright (c) 2000-2002 CollabNet. All rights reserved.
>>+ *
>>+ * This software is licensed as described in the file COPYING, which
>>+ * you should have received as part of this distribution. The terms
>>+ * are also available at http://subversion.tigris.org/license-1.html.
>>+ * If newer versions of this license are posted there, you may use a
>>+ * newer version instead, at your option.
>>+ *
>>+ * This software consists of voluntary contributions made by many
>>+ * individuals. For exact contribution history, see the revision
>>+ * history and logs, available at http://subversion.tigris.org/.
>>+ * ====================================================================
>>+ */
>>+
>>+
>>+
>>+#include "svn_error.h"
>>+#include "svn_pools.h"
>>+
>>+
>>+/*** Code. ***/
>>+
>>+static volatile sig_atomic_t canceled = FALSE;
>
>
> Is canceled an Americanism? I'd have spelt it cancelled.
it's more of a "garrett can't spell"ism ;-)
>>+
>>+static svn_boolean_t cancelation_in_progress = FALSE;
>
>
> Why the second flag?
i think it was because i was running into cases where we'd be canceled,
more than once. you'd hit control-c, get the first cancelation, and
then do an SVN_ERR during cleanup and get a second. i might be on crack
though, because it now occurs to me that another bug i fixed in this
code might have been causing that behavior. i'll have to test some more
when i get back to this.
>>+
>>+static apr_pool_t *cancelation_pool;
>>+
>>+svn_boolean_t
>>+svn__async_canceled(void)
>>+{
>>+ return canceled;
>>+}
>>+
>>+svn_error_t *
>>+svn__async_cancelation_error(void)
>>+{
>>+ canceled = FALSE;
>>+
>>+ cancelation_in_progress = TRUE;
>>+
>>+ return svn_error_create (SVN_ERR_CANCELED, 0, NULL,
>>cancelation_pool, NULL);
>>
>>+}
>>+
>>+svn_boolean_t
>>+svn__async_cancelation_in_progress (void)
>>+{
>>+ return cancelation_in_progress;
>>+}
>>+
>>+void
>>+svn_async_register_cancelation_pool (apr_pool_t *pool)
>>+{
>>+ cancelation_pool = pool;
>>+}
>>+
>>+void
>>+svn_async_cancel(void)
>>+{
>>+ if (! cancelation_in_progress)
>
>
> Given that this is a signal handler, the above test looks a little
> suspicious. Is it necessary? Can't you just set cancelled
> unconditionally? I'm not sure why you need two flags anyway, doesn't
> cancelled provide enough state?
again, i think you might be right. i think the problem that led me to
the two flags may have been something else which i've already fixed.
thanks for the comments. i will take them into account tonight or
tomorrow when i can get back to this.
-garrett
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: permissions (and other) problems
Posted by Philip Martin <ph...@codematters.co.uk>.
Garrett Rooney <ro...@electricjellyfish.net> writes:
> Index: subversion/include/svn_error.h
> ===================================================================
> --- subversion/include/svn_error.h
> +++ subversion/include/svn_error.h Thu Sep 12 22:49:39 2002
[snip]
> @@ -151,6 +175,8 @@
> svn_error_t *svn_err__temp = (expr); \
> if (svn_err__temp) \
> return svn_err__temp; \
> + else if (svn__async_canceled ()) \
> + return svn__async_cancelation_error (); \
Generating the error by calling a function will mean that the
debugging line number associated with the error will refer to that
function. If you called svn_error_create directly would mean that the
line number would refer to the SVN_ERR macro, which is much more
interesting, although it may increase the code size.
> } while (0)
>
>
> Index: subversion/libsvn_subr/cancel.c
> ===================================================================
> --- subversion/libsvn_subr/cancel.c
> +++ subversion/libsvn_subr/cancel.c Thu Sep 12 22:36:31 2002
> @@ -0,0 +1,80 @@
> +/*
> + * cancel.c: support for asyncronous cancelation of subversion functions
> + *
> + * ====================================================================
> + * Copyright (c) 2000-2002 CollabNet. All rights reserved.
> + *
> + * This software is licensed as described in the file COPYING, which
> + * you should have received as part of this distribution. The terms
> + * are also available at http://subversion.tigris.org/license-1.html.
> + * If newer versions of this license are posted there, you may use a
> + * newer version instead, at your option.
> + *
> + * This software consists of voluntary contributions made by many
> + * individuals. For exact contribution history, see the revision
> + * history and logs, available at http://subversion.tigris.org/.
> + * ====================================================================
> + */
> +
> +
> +
> +#include "svn_error.h"
> +#include "svn_pools.h"
> +
> +
> +/*** Code. ***/
> +
> +static volatile sig_atomic_t canceled = FALSE;
Is canceled an Americanism? I'd have spelt it cancelled.
> +
> +static svn_boolean_t cancelation_in_progress = FALSE;
Why the second flag?
> +
> +static apr_pool_t *cancelation_pool;
> +
> +svn_boolean_t
> +svn__async_canceled(void)
> +{
> + return canceled;
> +}
> +
> +svn_error_t *
> +svn__async_cancelation_error(void)
> +{
> + canceled = FALSE;
> +
> + cancelation_in_progress = TRUE;
> +
> + return svn_error_create (SVN_ERR_CANCELED, 0, NULL,
> cancelation_pool, NULL);
>
> +}
> +
> +svn_boolean_t
> +svn__async_cancelation_in_progress (void)
> +{
> + return cancelation_in_progress;
> +}
> +
> +void
> +svn_async_register_cancelation_pool (apr_pool_t *pool)
> +{
> + cancelation_pool = pool;
> +}
> +
> +void
> +svn_async_cancel(void)
> +{
> + if (! cancelation_in_progress)
Given that this is a signal handler, the above test looks a little
suspicious. Is it necessary? Can't you just set cancelled
unconditionally? I'm not sure why you need two flags anyway, doesn't
cancelled provide enough state?
> + canceled = TRUE;
> +}
--
Philip Martin
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: permissions (and other) problems
Posted by Garrett Rooney <ro...@electricjellyfish.net>.
Philip Martin wrote:
> Almost everywhere that ignores an error should use svn_error_clear_all,
> some places don't but that's usually a bug. We could change
>
> if (err)
> svn_error_clear_all (err);
>
> into
>
> if (err)
> SVN_ERR (svn_error_clear_all (err));
>
> and have svn_error_clear_all return an input ERR_CANCELED instead of
> clearing it.
>
yeah, that makes sense to me.
here's a first cut of this that i put together last night.
it uses functions inside SVN_ERR and SVN_ERR_W to figure out if we've
been canceled, rather than global variables, since for some reason no
matter what combination of declarations i tried, i wasn't able to get
everything to link without unresolved symbols.
there are actually two states we need to worry about. the first is
'have we been canceled', which tells SVN_ERR and SVN_ERR_W to return an
SVN_ERR_CANCELED error, and the second is 'are we in the process of
backing out because we received a cancelation', which causes
svn_error_clear_all to return an SVN_ERR_CANCELED error.
i can't for the life of me recall why i required these two states... i
wrote this code very late at night, and my memory of my reasoning is a
bit fuzzy. i think i saw some odd results when i was simply having
SVN_ERR unconditionally return an SVN_ERR_CANCELED error once we have
been canceled, but i forget the details.
in any event, this seems to work reaonsably well. i was able to cancel
safely out of everything i tried. the client app should probably be
checking for a SVN_ERR_CANCELED error and altering its output, since
printing out what is potentially a chain of uninteresting errors when
the user hit control-c is not really useful.
this patch is available at
http://electricjellyfish.net/garrett/code/cancel.diff since i suspect
mozilla's mailer will munge it...
note that i don't plan on actually committing this without a bunch more
work. the thread safety issues need to be considered, and a fair amount
of other things need to be cleaned up, but here's what i've got so far.
-garrett
Index: subversion/include/svn_error_codes.h
===================================================================
--- subversion/include/svn_error_codes.h
+++ subversion/include/svn_error_codes.h Thu Sep 12 19:29:06 2002
@@ -643,6 +643,10 @@
SVN_ERR_MISC_CATEGORY_START + 12,
"Error calling external program")
+ SVN_ERRDEF (SVN_ERR_CANCELED,
+ SVN_ERR_MISC_CATEGORY_START + 13,
+ "Operation was canceled")
+
/* command-line client errors */
SVN_ERRDEF (SVN_ERR_CL_ARG_PARSING_ERROR,
Index: subversion/include/svn_error.h
===================================================================
--- subversion/include/svn_error.h
+++ subversion/include/svn_error.h Thu Sep 12 22:49:39 2002
@@ -117,7 +117,7 @@
/* Clear ERROR's pool. Note that this is likely the top-level error
pool shared with any other errors currently extant, though usually
only one error exists at a time, so this is not a problem. */
-void svn_error_clear_all (svn_error_t *error);
+svn_error_t *svn_error_clear_all (svn_error_t *error);
/* Very basic default error handler: print out error stack, and quit
@@ -130,6 +130,30 @@
which must be passed in DATA. Allocates from a subpool of POOL. */
void svn_handle_warning (apr_pool_t *pool, void *data, const char
*fmt, ...);
+/* XXX i doubt these should go here. maybe an svn_cancel.h? */
+
+/* the next three prototypes are an implementation detail...
+ these are not the functions you are looking for...
+ move along, move along... */
+
+/* has the operation currently in progress been canceled by the client
app? */
+svn_boolean_t svn__async_canceled (void);
+
+/* generate an SVN_ERR_CANCEL error allocated from the previously
registered
+ cancelation pool */
+svn_error_t *svn__async_cancelation_error (void);
+
+/* are we currenlty cleaning up from a cancelation? */
+svn_boolean_t svn__async_cancelation_in_progress (void);
+
+/* register a pool to be used to allocate SVN_ERR_CANCEL errors. */
+void svn_async_register_cancelation_pool (apr_pool_t *pool);
+
+/* cancel the currently in progress action. */
+void svn_async_cancel (void);
+
+/* reset the global cancelation flags after a cancelation has occured. */
+void svn_async_reset (void);
/* A statement macro for checking error return values.
Evaluate EXPR. If it yields an error, return that error from the
@@ -151,6 +175,8 @@
svn_error_t *svn_err__temp = (expr); \
if (svn_err__temp) \
return svn_err__temp; \
+ else if (svn__async_canceled ()) \
+ return svn__async_cancelation_error (); \
} while (0)
@@ -162,6 +188,8 @@
svn_error_t *svn_err__temp = (expr); \
if (svn_err__temp) \
return svn_error_quick_wrap(svn_err__temp, wrap_msg); \
+ else if (svn__async_canceled ()) \
+ return svn__async_cancelation_error (); \
} while (0)
Index: subversion/libsvn_fs/tree.c
===================================================================
--- subversion/libsvn_fs/tree.c
+++ subversion/libsvn_fs/tree.c Thu Sep 12 21:45:12 2002
@@ -501,7 +501,7 @@
said it was optional, then don't return an error;
just put a NULL node pointer in the path. */
- svn_error_clear_all (err);
+ SVN_ERR (svn_error_clear_all (err));
if ((flags & open_path_last_optional)
&& (! next || *next == '\0'))
@@ -2345,7 +2345,7 @@
if (youngest_rev == youngish_rev)
return err;
else
- svn_error_clear_all (err);
+ SVN_ERR (svn_error_clear_all (err));
}
else if (err)
return err;
Index: subversion/libsvn_wc/log.c
===================================================================
--- subversion/libsvn_wc/log.c
+++ subversion/libsvn_wc/log.c Thu Sep 12 21:48:00 2002
@@ -626,7 +626,7 @@
/* It's possible that locally modified files were left behind during
the removal. That's okay; just check for this special case. */
if (err && (err->apr_err == SVN_ERR_WC_LEFT_LOCAL_MOD))
- svn_error_clear_all (err);
+ SVN_ERR (svn_error_clear_all (err));
else if (err)
return err;
Index: subversion/libsvn_wc/adm_ops.c
===================================================================
--- subversion/libsvn_wc/adm_ops.c
+++ subversion/libsvn_wc/adm_ops.c Thu Sep 12 21:47:00 2002
@@ -723,7 +723,7 @@
err = svn_wc_entry (&orig_entry, path, TRUE, pool);
if (err)
{
- svn_error_clear_all (err);
+ SVN_ERR (svn_error_clear_all (err));
orig_entry = NULL;
}
@@ -1550,7 +1550,7 @@
if (err && (err->apr_err == SVN_ERR_WC_LEFT_LOCAL_MOD))
{
- svn_error_clear_all (err);
+ SVN_ERR (svn_error_clear_all (err));
left_something = TRUE;
}
else if (err)
@@ -1570,7 +1570,7 @@
destroy_wf,
subpool);
if (err && (err->apr_err == SVN_ERR_WC_LEFT_LOCAL_MOD))
{
- svn_error_clear_all (err);
+ SVN_ERR (svn_error_clear_all (err));
left_something = TRUE;
}
else if (err)
Index: subversion/libsvn_wc/status.c
===================================================================
--- subversion/libsvn_wc/status.c
+++ subversion/libsvn_wc/status.c Thu Sep 12 21:48:38 2002
@@ -554,7 +554,7 @@
if (svn_err->apr_err !=
SVN_ERR_WC_NOT_DIRECTORY)
return svn_err;
- svn_error_clear_all (svn_err);
+ SVN_ERR (svn_error_clear_all (svn_err));
fullpath_entry = entry;
}
}
Index: subversion/libsvn_wc/adm_files.c
===================================================================
--- subversion/libsvn_wc/adm_files.c
+++ subversion/libsvn_wc/adm_files.c Thu Sep 12 21:46:27 2002
@@ -1020,7 +1020,7 @@
if (err)
{
- svn_error_clear_all (err);
+ SVN_ERR (svn_error_clear_all (err));
wc_exists = FALSE;
}
else if (wc_format > SVN_WC__VERSION)
Index: subversion/libsvn_wc/update_editor.c
===================================================================
--- subversion/libsvn_wc/update_editor.c
+++ subversion/libsvn_wc/update_editor.c Thu Sep 12 21:49:02 2002
@@ -1118,7 +1118,7 @@
}
else if (err)
{
- svn_error_clear_all (err);
+ SVN_ERR (svn_error_clear_all (err));
hb->source = NULL; /* make sure */
}
}
@@ -2132,7 +2132,7 @@
if (err || (! p_entry))
{
if (err)
- svn_error_clear_all (err);
+ SVN_ERR (svn_error_clear_all (err));
return SVN_NO_ERROR;
}
Index: subversion/libsvn_wc/lock.c
===================================================================
--- subversion/libsvn_wc/lock.c
+++ subversion/libsvn_wc/lock.c Thu Sep 12 21:47:35 2002
@@ -86,7 +86,7 @@
{
if (APR_STATUS_IS_EEXIST(err->apr_err))
{
- svn_error_clear_all (err);
+ SVN_ERR (svn_error_clear_all (err));
if (wait_for <= 0)
break;
wait_for--;
@@ -135,7 +135,7 @@
if (err)
{
apr_status_t apr_err = err->apr_err;
- svn_error_clear_all (err);
+ SVN_ERR (svn_error_clear_all (err));
return apr_err;
}
else
@@ -198,7 +198,7 @@
if (err)
{
if (err->apr_err == SVN_ERR_WC_LOCKED)
- svn_error_clear_all (err); /* Steal existing lock */
+ SVN_ERR (svn_error_clear_all (err)); /* Steal existing lock */
else
return err;
}
Index: subversion/libsvn_wc/questions.c
===================================================================
--- subversion/libsvn_wc/questions.c
+++ subversion/libsvn_wc/questions.c Thu Sep 12 21:48:20 2002
@@ -74,7 +74,7 @@
wrong at all, then for our purposes this is not a working
copy, so return 0. */
if (err)
- svn_error_clear_all (err);
+ SVN_ERR (svn_error_clear_all (err));
*wc_format = 0;
}
Index: subversion/libsvn_subr/svn_error.c
===================================================================
--- subversion/libsvn_subr/svn_error.c
+++ subversion/libsvn_subr/svn_error.c Thu Sep 12 23:04:45 2002
@@ -305,10 +305,16 @@
}
-void
+svn_error_t *
svn_error_clear_all (svn_error_t *err)
{
- svn_pool_clear (err->pool);
+ if (svn__async_cancelation_in_progress ())
+ return svn__async_cancelation_error ();
+ else
+ {
+ svn_pool_clear (err->pool);
+ return SVN_NO_ERROR;
+ }
}
Index: subversion/libsvn_subr/cancel.c
===================================================================
--- subversion/libsvn_subr/cancel.c
+++ subversion/libsvn_subr/cancel.c Thu Sep 12 22:36:31 2002
@@ -0,0 +1,80 @@
+/*
+ * cancel.c: support for asyncronous cancelation of subversion functions
+ *
+ * ====================================================================
+ * Copyright (c) 2000-2002 CollabNet. All rights reserved.
+ *
+ * This software is licensed as described in the file COPYING, which
+ * you should have received as part of this distribution. The terms
+ * are also available at http://subversion.tigris.org/license-1.html.
+ * If newer versions of this license are posted there, you may use a
+ * newer version instead, at your option.
+ *
+ * This software consists of voluntary contributions made by many
+ * individuals. For exact contribution history, see the revision
+ * history and logs, available at http://subversion.tigris.org/.
+ * ====================================================================
+ */
+
+
+
+#include "svn_error.h"
+#include "svn_pools.h"
+
+
+/*** Code. ***/
+
+static volatile sig_atomic_t canceled = FALSE;
+
+static svn_boolean_t cancelation_in_progress = FALSE;
+
+static apr_pool_t *cancelation_pool;
+
+svn_boolean_t
+svn__async_canceled(void)
+{
+ return canceled;
+}
+
+svn_error_t *
+svn__async_cancelation_error(void)
+{
+ canceled = FALSE;
+
+ cancelation_in_progress = TRUE;
+
+ return svn_error_create (SVN_ERR_CANCELED, 0, NULL, cancelation_pool,
NULL);
+}
+
+svn_boolean_t
+svn__async_cancelation_in_progress (void)
+{
+ return cancelation_in_progress;
+}
+
+void
+svn_async_register_cancelation_pool (apr_pool_t *pool)
+{
+ cancelation_pool = pool;
+}
+
+void
+svn_async_cancel(void)
+{
+ if (! cancelation_in_progress)
+ canceled = TRUE;
+}
+
+void
+svn_async_reset(void)
+{
+ canceled = FALSE;
+
+ cancelation_in_progress = FALSE;
+}
+
+
+/*
+ * local variables:
+ * eval: (load-file "../../tools/dev/svn-dev.el")
+ * end: */
Index: subversion/libsvn_client/commit_util.c
===================================================================
--- subversion/libsvn_client/commit_util.c
+++ subversion/libsvn_client/commit_util.c Thu Sep 12 21:44:48 2002
@@ -148,7 +148,7 @@
err = svn_wc_entries_read (&entries, path, FALSE, subpool);
if (err)
{
- svn_error_clear_all (err);
+ SVN_ERR (svn_error_clear_all (err));
entries = NULL;
}
Index: subversion/mod_dav_svn/repos.c
===================================================================
--- subversion/mod_dav_svn/repos.c
+++ subversion/mod_dav_svn/repos.c Thu Sep 12 21:49:23 2002
@@ -1487,7 +1487,7 @@
resource->pool);
if (serr != NULL && serr->apr_err == SVN_ERR_FS_NOT_FOUND)
{
- svn_error_clear_all(serr);
+ SVN_ERR (svn_error_clear_all(serr));
serr = svn_fs_make_file(resource->info->root.root,
resource->info->repos_path,
resource->pool);
Index: subversion/clients/cmdline/add-cmd.c
===================================================================
--- subversion/clients/cmdline/add-cmd.c
+++ subversion/clients/cmdline/add-cmd.c Thu Sep 12 21:49:49 2002
@@ -71,7 +71,7 @@
if (err->apr_err == SVN_ERR_ENTRY_EXISTS)
{
svn_handle_warning (err->pool, stderr, err->message);
- svn_error_clear_all (err);
+ SVN_ERR (svn_error_clear_all (err));
}
else
return err;
Index: subversion/clients/cmdline/resolve-cmd.c
===================================================================
--- subversion/clients/cmdline/resolve-cmd.c
+++ subversion/clients/cmdline/resolve-cmd.c Thu Sep 12 21:50:08 2002
@@ -68,7 +68,7 @@
if (err)
{
svn_handle_warning (err->pool, stderr, err->message);
- svn_error_clear_all (err);
+ SVN_ERR (svn_error_clear_all (err));
}
svn_pool_clear (subpool);
Index: subversion/clients/cmdline/main.c
===================================================================
--- subversion/clients/cmdline/main.c
+++ subversion/clients/cmdline/main.c Thu Sep 12 21:20:14 2002
@@ -873,6 +873,12 @@
}
+static void
+cancel (int sig)
+{
+ svn_async_cancel();
+}
+
/*** Main. ***/
@@ -913,6 +919,11 @@
/* Create our top-level pool. */
pool = svn_pool_create (NULL);
+
+ /* XXX where should this go? */
+ svn_async_register_cancelation_pool (pool);
+
+ signal(SIGINT, cancel);
/* Begin processing arguments. */
memset (&opt_state, 0, sizeof (opt_state));
Index: subversion/libsvn_repos/delta.c
===================================================================
--- subversion/libsvn_repos/delta.c
+++ subversion/libsvn_repos/delta.c Thu Sep 12 21:46:06 2002
@@ -266,7 +266,7 @@
{
/* Caller thinks that target still exists, but it doesn't.
So just delete the target and go home. */
- svn_error_clear_all (err);
+ SVN_ERR (svn_error_clear_all (err));
SVN_ERR (delete (&c, root_baton, src_entry, pool));
goto cleanup;
}
@@ -282,7 +282,7 @@
{
/* The target has been deleted from our working copy. Add
back a new one. */
- svn_error_clear_all (err);
+ SVN_ERR (svn_error_clear_all (err));
SVN_ERR (add_file_or_dir (&c, root_baton,
tgt_parent_dir->data,
tgt_entry->data,
Index: subversion/libsvn_ra_dav/commit.c
===================================================================
--- subversion/libsvn_ra_dav/commit.c
+++ subversion/libsvn_ra_dav/commit.c Thu Sep 12 21:45:31 2002
@@ -972,7 +972,7 @@
{
/* ### TODO: This is what we get if the file doesn't exist
but an explicit not-found error might be better */
- svn_error_clear_all(err);
+ SVN_ERR (svn_error_clear_all(err));
}
else
{
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: permissions (and other) problems
Posted by Philip Martin <ph...@codematters.co.uk>.
Branko Äibej <br...@xbc.nu> writes:
> > i'm leaning towards having the client call svn_async_clear, since
> > otherwise, you could get in the situation where SVN_ERR reset the
> > flag, but the error that was returned was ignored by something
> > further up the call chain (but still inside an svn function). i
> > believe there are places where we do ignore errors explicitly, so it
> > could theoretically happen...
>
> If SVN_ERR detects a cancelation, it must return an ERR_CANCELED error
> (perhaps wrapping the original error in that, but maybe not). That
> implies that SVN_ERR should check if the incomng error code is already
> ERR_CANCELED. It also implies that all places where we migh ignore
> errors have to be checked; cancellation shouldn't be ignored.
Well, using svn_async_cancel/svn_async_clear once the client calls
cancel it doesn't make much sense to call clear until all the
Subversion functions (that's in all threads) have returned. So if an
ERR_CANCELED error gets dropped, the next SVN_ERR will generate
another.
There is some code that doesn't return an svn_error_t* at all, the
notify callback for example, where there is no option but to drop any
ERR_CANCELED.
> Sure, that's a bit more work than just adding the cancellation error...
Almost everywhere that ignores an error should use svn_error_clear_all,
some places don't but that's usually a bug. We could change
if (err)
svn_error_clear_all (err);
into
if (err)
SVN_ERR (svn_error_clear_all (err));
and have svn_error_clear_all return an input ERR_CANCELED instead of
clearing it.
--
Philip Martin
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: permissions (and other) problems
Posted by Branko Čibej <br...@xbc.nu>.
Garrett Rooney wrote:
> On Thursday, September 12, 2002, at 05:34 PM, Philip Martin wrote:
>
>> Garrett Rooney <ro...@electricjellyfish.net> writes:
>>
>>>> Either way, the interaction with a multi-threaded Subversion client
>>>> needs to be considered, even if it's only to document what happens.
>>>> E.g. to say that svn_set_cancelation_handler affects all threads or
>>>> that svn_async_cancel affects one thread.
>>>
>>>
>>> that's a tough question. i'm tempted to say that svn_async_cancel
>>> effects all threads, but i imagine it might be nice to have it only
>>> effect the current thread...
>>
>>
>> Assuming svn_async_cancel sets some global state, probably just a
>> flag, the question then is when does it get reset? Does the SVN_ERR
>> macro reset it after deciding to generate an error? Does the client
>> have to call svn_async_clear?
>
>
> i'm leaning towards having the client call svn_async_clear, since
> otherwise, you could get in the situation where SVN_ERR reset the
> flag, but the error that was returned was ignored by something further
> up the call chain (but still inside an svn function). i believe there
> are places where we do ignore errors explicitly, so it could
> theoretically happen...
If SVN_ERR detects a cancelation, it must return an ERR_CANCELED error
(perhaps wrapping the original error in that, but maybe not). That
implies that SVN_ERR should check if the incomng error code is already
ERR_CANCELED. It also implies that all places where we migh ignore
errors have to be checked; cancellation shouldn't be ignored.
Sure, that's a bit more work than just adding the cancellation error...
--
Brane Čibej <br...@xbc.nu> http://www.xbc.nu/brane/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: permissions (and other) problems
Posted by Garrett Rooney <ro...@electricjellyfish.net>.
On Thursday, September 12, 2002, at 05:34 PM, Philip Martin wrote:
> Garrett Rooney <ro...@electricjellyfish.net> writes:
>
>>> Either way, the interaction with a multi-threaded Subversion client
>>> needs to be considered, even if it's only to document what happens.
>>> E.g. to say that svn_set_cancelation_handler affects all threads or
>>> that svn_async_cancel affects one thread.
>>
>> that's a tough question. i'm tempted to say that svn_async_cancel
>> effects all threads, but i imagine it might be nice to have it only
>> effect the current thread...
>
> Assuming svn_async_cancel sets some global state, probably just a
> flag, the question then is when does it get reset? Does the SVN_ERR
> macro reset it after deciding to generate an error? Does the client
> have to call svn_async_clear?
i'm leaning towards having the client call svn_async_clear, since
otherwise, you could get in the situation where SVN_ERR reset the flag,
but the error that was returned was ignored by something further up the
call chain (but still inside an svn function). i believe there are
places where we do ignore errors explicitly, so it could theoretically
happen...
-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
Re: permissions (and other) problems
Posted by Philip Martin <ph...@codematters.co.uk>.
Garrett Rooney <ro...@electricjellyfish.net> writes:
> > Either way, the interaction with a multi-threaded Subversion client
> > needs to be considered, even if it's only to document what happens.
> > E.g. to say that svn_set_cancelation_handler affects all threads or
> > that svn_async_cancel affects one thread.
>
> that's a tough question. i'm tempted to say that svn_async_cancel
> effects all threads, but i imagine it might be nice to have it only
> effect the current thread...
Assuming svn_async_cancel sets some global state, probably just a
flag, the question then is when does it get reset? Does the SVN_ERR
macro reset it after deciding to generate an error? Does the client
have to call svn_async_clear?
> of course, threads and signals don't
> work too well together anyway...
They work if you follow the rules :)
--
Philip Martin
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: permissions (and other) problems
Posted by Garrett Rooney <ro...@electricjellyfish.net>.
Philip Martin wrote:
> Garrett Rooney <ro...@electricjellyfish.net> writes:
>
>
>>I like this idea.
>>
>>The client calls some function (svn_set_cancelation_handler() ?) which
>>sets the function to be called by SVN_ERR(). then SVN_ERR() can call
>>it (if it's been set) and if so, it returns the error.
>>
>>
>>We can't just have a global variable, it's got to be something that
>>can be controlled by the client application, but other than that i
>>really like this idea.
>
>
> Another way would be to provide a function that the client
> application's signal handler calls to indicate that a cancellation
> should occur, e.g. have svn_async_cancel() called by the signal
> handler rather than svn_set_cancelation_handler() by the main
> code. That might make for a more efficient SVN_ERR implementation.
good idea! i like that a lot more than calling a user defined function
every time we use SVN_ERR().
> Either way, the interaction with a multi-threaded Subversion client
> needs to be considered, even if it's only to document what happens.
> E.g. to say that svn_set_cancelation_handler affects all threads or
> that svn_async_cancel affects one thread.
that's a tough question. i'm tempted to say that svn_async_cancel
effects all threads, but i imagine it might be nice to have it only
effect the current thread... of course, threads and signals don't work
too well together anyway...
any opinions?
-garrett
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: permissions (and other) problems
Posted by Philip Martin <ph...@codematters.co.uk>.
Garrett Rooney <ro...@electricjellyfish.net> writes:
> I like this idea.
>
> The client calls some function (svn_set_cancelation_handler() ?) which
> sets the function to be called by SVN_ERR(). then SVN_ERR() can call
> it (if it's been set) and if so, it returns the error.
>
>
> We can't just have a global variable, it's got to be something that
> can be controlled by the client application, but other than that i
> really like this idea.
Another way would be to provide a function that the client
application's signal handler calls to indicate that a cancellation
should occur, e.g. have svn_async_cancel() called by the signal
handler rather than svn_set_cancelation_handler() by the main
code. That might make for a more efficient SVN_ERR implementation.
Either way, the interaction with a multi-threaded Subversion client
needs to be considered, even if it's only to document what happens.
E.g. to say that svn_set_cancelation_handler affects all threads or
that svn_async_cancel affects one thread.
--
Philip Martin
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: permissions (and other) problems
Posted by Garrett Rooney <ro...@electricjellyfish.net>.
Branko Čibej wrote:
> Cool. I agree a function is better (although slower, but let's not
> optimize without perf measurements). I'll be away all week from Saturday
> (sailing again, whee), and I fully expect to see this mechanism in the
> code when I get back. Hint, hint! :-)
i'll try and cook something up over the weekend ;-)
-garrett
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: permissions (and other) problems
Posted by Branko Čibej <br...@xbc.nu>.
Garrett Rooney wrote:
> Branko Čibej wrote:
>
>> However, I think I we already have a perfect hook where we can check
>> for cancellation. It's called SVN_ERR. Just introduce a new error
>> code, SVN_CANCELED, and have the SVN_ERR macro check for cancellation
>> (a global var) and return an svn_error_t with that code,
>>
>> The command-line client would create a SIGINT handler that set that
>> var, a GUI client would do something else.
>
>
> I like this idea.
>
> The client calls some function (svn_set_cancelation_handler() ?) which
> sets the function to be called by SVN_ERR(). then SVN_ERR() can call
> it (if it's been set) and if so, it returns the error.
>
> We can't just have a global variable, it's got to be something that
> can be controlled by the client application, but other than that i
> really like this idea.
Cool. I agree a function is better (although slower, but let's not
optimize without perf measurements). I'll be away all week from Saturday
(sailing again, whee), and I fully expect to see this mechanism in the
code when I get back. Hint, hint! :-)
--
Brane Čibej <br...@xbc.nu> http://www.xbc.nu/brane/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: permissions (and other) problems
Posted by Garrett Rooney <ro...@electricjellyfish.net>.
Branko Čibej wrote:
> mark benedetto king wrote:
>
>> On Thu, Sep 12, 2002 at 10:09:49AM -0400, Garrett Rooney wrote:
>>
>>> To provide a means for the client application to indicate that it
>>> wants the library to stop whatever it was doing and clean up.
>>>
>>
>> Yes, but then you need to check for the cancellation every so often,
>> which can be painful.
>>
>> Also, if the application is actually blocked on a long-running operation
>> (maybe the database is already wedged, for example), this approach won't
>> work. Maybe BDB provides some asynchronous accessors...
>>
> Nope.
>
> However, I think I we already have a perfect hook where we can check for
> cancellation. It's called SVN_ERR. Just introduce a new error code,
> SVN_CANCELED, and have the SVN_ERR macro check for cancellation (a
> global var) and return an svn_error_t with that code,
>
> The command-line client would create a SIGINT handler that set that var,
> a GUI client would do something else.
I like this idea.
The client calls some function (svn_set_cancelation_handler() ?) which
sets the function to be called by SVN_ERR(). then SVN_ERR() can call it
(if it's been set) and if so, it returns the error.
We can't just have a global variable, it's got to be something that can
be controlled by the client application, but other than that i really
like this idea.
-garrett
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: permissions (and other) problems
Posted by Branko Čibej <br...@xbc.nu>.
mark benedetto king wrote:
>On Thu, Sep 12, 2002 at 10:09:49AM -0400, Garrett Rooney wrote:
>
>>To provide a means for the client application to indicate that it wants
>>the library to stop whatever it was doing and clean up.
>>
>
>Yes, but then you need to check for the cancellation every so often,
>which can be painful.
>
>Also, if the application is actually blocked on a long-running operation
>(maybe the database is already wedged, for example), this approach won't
>work. Maybe BDB provides some asynchronous accessors...
>
Nope.
However, I think I we already have a perfect hook where we can check for
cancellation. It's called SVN_ERR. Just introduce a new error code,
SVN_CANCELED, and have the SVN_ERR macro check for cancellation (a
global var) and return an svn_error_t with that code,
The command-line client would create a SIGINT handler that set that var,
a GUI client would do something else.
>>I seem to remember it being removed a while back (and i'm not sure why),
>>but it does seem like there needs to be some means of providing this
>>functionality, both for the command line client and control-c, and for a
>>gui client with a dialog where you can just hit cancel.
>>
A cancellation editor is just too cumbersome, and can't be used everywhere.
>Yes, it would be nice to have a client-independent solution.
>
>
That's exactly what we'd get with my proposal.
--
Brane Čibej <br...@xbc.nu> http://www.xbc.nu/brane/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: permissions (and other) problems
Posted by mark benedetto king <bk...@inquira.com>.
On Thu, Sep 12, 2002 at 10:09:49AM -0400, Garrett Rooney wrote:
> > I posted a kludgey patch to prevent that a few months ago;
> > it just ignored the SIGINT. It did this even on ra_dav, though, which
> > was probably overkill.
> >
> > Maybe the work should have been done in svn_ra_local_init().
> > What do you think?
>
> Isn't this the kind of situation that the cancelation editor that Bill
> Tutt wrote a while back would be used for?
>
Right. You could have something like
static svn_boolean_t cancelled = FALSE;
static void
siginthandler(int sig)
{
cancelled = TRUE;
}
svn_boolean_t
is_cancelled() {
return cancelled;
}
and just pass "is_cancelled" as a client callback to the cancellable
functions.
I strongly believe that the client API needs to be widened to support
a more easily extensible callback mechanism; right now, most callbacks
are passed as parameters (rather than in a structure, like the
auth_baton). This means that every callback we want to add will have
ripple-effects through the code, and break the published client API.
> To provide a means for the client application to indicate that it wants
> the library to stop whatever it was doing and clean up.
>
Yes, but then you need to check for the cancellation every so often,
which can be painful.
Also, if the application is actually blocked on a long-running operation
(maybe the database is already wedged, for example), this approach won't
work. Maybe BDB provides some asynchronous accessors...
> I seem to remember it being removed a while back (and i'm not sure why),
> but it does seem like there needs to be some means of providing this
> functionality, both for the command line client and control-c, and for a
> gui client with a dialog where you can just hit cancel.
Yes, it would be nice to have a client-independent solution.
--ben
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: permissions (and other) problems
Posted by Garrett Rooney <ro...@electricjellyfish.net>.
mark benedetto king wrote:
> On Thu, Sep 12, 2002 at 08:19:31AM +0200, brane@xbc.nu wrote:
>
>>Apart from the restriction that the repository must be on a local disk (e.g.,
>>"local" access over NFS is impossible)? The permission problems may well have
>>been causing the lock-ups; the other p[robable cause interrupting svn (^C) while
>>it's fiddling with the repository (aargh, we really, really should find a way to
>>avoid fix that).
>>
>
>
> I posted a kludgey patch to prevent that a few months ago;
> it just ignored the SIGINT. It did this even on ra_dav, though, which
> was probably overkill.
>
> Maybe the work should have been done in svn_ra_local_init().
> What do you think?
Isn't this the kind of situation that the cancelation editor that Bill
Tutt wrote a while back would be used for?
To provide a means for the client application to indicate that it wants
the library to stop whatever it was doing and clean up.
I seem to remember it being removed a while back (and i'm not sure why),
but it does seem like there needs to be some means of providing this
functionality, both for the command line client and control-c, and for a
gui client with a dialog where you can just hit cancel.
-garrett
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: permissions (and other) problems
Posted by Branko Čibej <br...@xbc.nu>.
Jon Watte wrote:
>>Note the the longjmp prevents transaction()'s mutex_exit() from being
>>called. This means that when the cleanup routine calls close(), the
>>mutex_enter() will block. Badness.
>>
>>
>
>As I said, not every case can be handled 100% -- but I think trying
>is good.
>
Trying is totally stupid if you know in advance that you have no control
over whether you'll succeed or not. That's simply bad (over)engineering,
sorry. This is not about a moral commitment to do your best, it's about
building a reliable software API in the face of uncontrollable external
events. The correct approach is to find a simple and foolproof solution,
which we're doing now.
--
Brane Čibej <br...@xbc.nu> http://www.xbc.nu/brane/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
RE: permissions (and other) problems
Posted by Jon Watte <hp...@mindcontrol.org>.
> Note the the longjmp prevents transaction()'s mutex_exit() from being
> called. This means that when the cleanup routine calls close(), the
> mutex_enter() will block. Badness.
As I said, not every case can be handled 100% -- but I think trying
is good. If this is really the implementation, and there is no asynchronous
database API, then the database API is, uh, in need of improvement. Neither
error codes nor signal handlers are going to make ctrl-C clean up nicely.
Now, if you're worried about the acquisition count mis-match blocking
forever (as you should be) then alarm() might "solve" that, but you'd
still end up with a hung DB at the end -- ergo, DB needs to be re-designed
to support the appropriate semantics, no matter what.
Anyway, the error code checking seems to be what's happening, and we
can hope that the DB is actually studlier than your example, so that a
ctrl-C terminates in reasonable time.
> Now, you'll argue that "mutex_exit()" should be added to the cleanup-list
> in each of these functions. That's true, but these functions are part
> of the database implementation, not part of Subversion. We might even be
> able to patch BDB but we can't patch Oracle.
Luckily, Oracle has an API that is more expressive than your example.
It has other problems, though :-)
Cheers,
/ h+
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: permissions (and other) problems
Posted by mark benedetto king <bk...@inquira.com>.
On Thu, Sep 12, 2002 at 07:57:17PM -0700, Jon Watte wrote:
>
> > There's also the small problem that longjmp'ing out of a signal handler
> > loses context, which means that we can't clean up the database locks
> > anyway -- defeating the purpose of this exercice.
>
> Note that my proposal was that anyone who enters context, also registers
> a cleanup handler & magic token for this context. When you're done with
> your protected area, you un-register the context. This is somewhat
> analogous to creating a memory pool and passing it "downstream" and then
> de-allocating the entire pool when you're done, which I believe subversion
> makes good use of by getting it from APR.
Unfortunately, it might not be safe to call the DB api to release locks
if you've longjmped out of a DB call in the first place, because the DB
might not play by this "register your cleanup routines" rule internally.
(Just like third party libs might call malloc() rather than apr_palloc()).
Take this hypothetical scenario:
open() {
mutex_enter();
obtain_lock();
mutex_exit();
}
transaction() {
mutex_enter();
do_long_blocking_thing();
mutex_exit();
}
close() {
mutex_enter();
release_lock();
mutex_exit();
}
Now, let's say we longjmp out of do_long_blocking_thing up to something
that calls close() to release the lock.
Note the the longjmp prevents transaction()'s mutex_exit() from being
called. This means that when the cleanup routine calls close(), the
mutex_enter() will block. Badness.
Now, you'll argue that "mutex_exit()" should be added to the cleanup-list
in each of these functions. That's true, but these functions are part
of the database implementation, not part of Subversion. We might even be
able to patch BDB but we can't patch Oracle.
--ben
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: permissions (and other) problems
Posted by Greg Stein <gs...@lyra.org>.
On Fri, Sep 13, 2002 at 01:13:57AM +0200, Branko Cibej wrote:
> Jon Watte wrote:
>...
> >While that is probably true, the benefit of using the signal handler
> >approach is that you can also catch SIGSEGV to do the same clean-up :-)
>
> There's also the small problem that longjmp'ing out of a signal handler
> loses context, which means that we can't clean up the database locks
> anyway -- defeating the purpose of this exercice.
>
> And trying to clean up after a seg faulr simply doesn't work in general.
Right. At one point, the Apache httpd grew a signal handler for SIGSEGV. One
of the hardcore geeks ripped a couple people a new one for that. There is
simply *no* effective response to a SIGSEGV. There is no way to clean up,
and there is even no way to issue a message to the user (no I/O at all). You
can't be sure data is around (the SEGV may have been an invalid access to
data), and all your memory and I/O structures could be toast.
Nope... just avoid trying to catch SIGSEGV. Die, and let your parent process
do something about it. (of course, that is usually the shell in terms of the
cmdline client :-)
Cheers,
-g
--
Greg Stein, http://www.lyra.org/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
RE: permissions (and other) problems
Posted by Jon Watte <hp...@mindcontrol.org>.
> There's also the small problem that longjmp'ing out of a signal handler
> loses context, which means that we can't clean up the database locks
> anyway -- defeating the purpose of this exercice.
Note that my proposal was that anyone who enters context, also registers
a cleanup handler & magic token for this context. When you're done with
your protected area, you un-register the context. This is somewhat
analogous to creating a memory pool and passing it "downstream" and then
de-allocating the entire pool when you're done, which I believe subversion
makes good use of by getting it from APR.
> And trying to clean up after a seg faulr simply doesn't work in general.
You'd think so, wouldn't you? However, it turns out to actually be quite
possible in real life; especially if all you want to do is to release some
resources (like database locks) and then exit in peace. (Note that I've
been back and forth on this issue two full circles in previous history.)
Yes, I understand that a user level program cannot make the same GUARANTEE
of correctness that a properly debugged kernel can for file descriptors,
but in reality, a segfault will be caused by some stupid bug dereferencing
NULL, or double-freeing memory, or some-souch; neither of which is likely
to thrash the data structures you have pre-allocated to be able to clean
up and go home. And as you don't re-install the SEGV handler (now THAT
would be pushing it!) in the 1-out-of-1000-th case that you can't clean up
then, well, you're no worse off than before.
I haven't yet seen an argument that convinces me a signal handler is not
the way to go. However, error codes will probably work as well for the
Ctrl-C case, and as y'all are already writing code and I'm not prepared to
spend the time to Do It My Way, then I should just shut up, shouldn't I? :-)
Cheers,
/ h+
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: permissions (and other) problems
Posted by Branko Čibej <br...@xbc.nu>.
Jon Watte wrote:
>>>A slightly less kludgey fix is to install a SIGINT handler which
>>>does a longjmp() to some routine which cleans up all currently
>>>registered "cleanup" handlers. Typically, something that starts
>>>a transaction will register transaction rollback as a cleanup task,
>>>and something which opens the database will register database close
>>>as a cleanup task...
>>>
>>>
>
>
>
>>No. We want to be portable, and doing anythint except setting a
>>sig_atomic_t global variable from within a signal handler is not portable.
>>
>>
>
>While that is probably true, the benefit of using the signal handler
>approach is that you can also catch SIGSEGV to do the same clean-up :-)
>
There's also the small problem that longjmp'ing out of a signal handler
loses context, which means that we can't clean up the database locks
anyway -- defeating the purpose of this exercice.
And trying to clean up after a seg faulr simply doesn't work in general.
--
Brane Čibej <br...@xbc.nu> http://www.xbc.nu/brane/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
RE: permissions (and other) problems
Posted by Jon Watte <hp...@mindcontrol.org>.
> >A slightly less kludgey fix is to install a SIGINT handler which
> >does a longjmp() to some routine which cleans up all currently
> >registered "cleanup" handlers. Typically, something that starts
> >a transaction will register transaction rollback as a cleanup task,
> >and something which opens the database will register database close
> >as a cleanup task...
> No. We want to be portable, and doing anythint except setting a
> sig_atomic_t global variable from within a signal handler is not portable.
While that is probably true, the benefit of using the signal handler
approach is that you can also catch SIGSEGV to do the same clean-up :-)
Regarding portability and practice: I've found longjumps from out of
signal handlers to be useable on all flavors of Windows, as well as
all flavors of UNIX I've used. Where else will subversion run that is
also a place where longjmp() out of signal handlers won't work?
(Don't count MacOS Classic, as that doesn't even have signals proper.)
Anyway, I just thought I'd offer this suggestion, as it often saves a
lot of work, and often can be made to be very robust. If you'd rather
be checking error codes, go right ahead.
Cheers,
/ h+
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: permissions (and other) problems
Posted by Branko Čibej <br...@xbc.nu>.
Jon Watte wrote:
>>I posted a kludgey patch to prevent that a few months ago;
>>it just ignored the SIGINT. It did this even on ra_dav, though, which
>>was probably overkill.
>>
>>
>
>A slightly less kludgey fix is to install a SIGINT handler which
>does a longjmp() to some routine which cleans up all currently
>registered "cleanup" handlers. Typically, something that starts
>a transaction will register transaction rollback as a cleanup task,
>and something which opens the database will register database close
>as a cleanup task...
>
No. We want to be portable, and doing anythint except setting a
sig_atomic_t global variable from within a signal handler is not portable.
The Berkeley DB utilities use a similar method to the one I proposed,
for the same reason, I assume.
>Note that Linux has problems fully restoring all CPU state when
>doing longjmp() out of signal handlers (the FPU control word is not
>correct) but that doesn't matter for this case :-)
>
>
That's exactly what I mean. Every system behaves slightly differently on
longjump, especially in a signal handler. Let's not play with fire if we
have a perefectly usable nuke handy. :-)
--
Brane Čibej <br...@xbc.nu> http://www.xbc.nu/brane/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
RE: permissions (and other) problems
Posted by Jon Watte <hp...@mindcontrol.org>.
> I posted a kludgey patch to prevent that a few months ago;
> it just ignored the SIGINT. It did this even on ra_dav, though, which
> was probably overkill.
A slightly less kludgey fix is to install a SIGINT handler which
does a longjmp() to some routine which cleans up all currently
registered "cleanup" handlers. Typically, something that starts
a transaction will register transaction rollback as a cleanup task,
and something which opens the database will register database close
as a cleanup task...
This way, you can reasonably abort a running operation, without
leaving the DB in a b0rked state.
Note that Linux has problems fully restoring all CPU state when
doing longjmp() out of signal handlers (the FPU control word is not
correct) but that doesn't matter for this case :-)
Cheers,
/ h+
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: permissions (and other) problems
Posted by mark benedetto king <bk...@inquira.com>.
On Thu, Sep 12, 2002 at 08:19:31AM +0200, brane@xbc.nu wrote:
>
> Apart from the restriction that the repository must be on a local disk (e.g.,
> "local" access over NFS is impossible)? The permission problems may well have
> been causing the lock-ups; the other p[robable cause interrupting svn (^C) while
> it's fiddling with the repository (aargh, we really, really should find a way to
> avoid fix that).
>
I posted a kludgey patch to prevent that a few months ago;
it just ignored the SIGINT. It did this even on ra_dav, though, which
was probably overkill.
Maybe the work should have been done in svn_ra_local_init().
What do you think?
--ben
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: permissions (and other) problems
Posted by Ben Collins-Sussman <su...@collab.net>.
Rich Drewes <dr...@interstice.com> writes:
> cmpilato@collab.net wrote:
>
> >Ben Collins-Sussman <su...@collab.net> writes:
> >
> >
> >>>This is on subversion-r2092g.tar.gz.
> >>>
> >>>Anyone know what is going on here?
> >>>
> >>svn r2092! Yeesh, that's from last *May*!
> >>
> Sorry, I goofed, this was actually:
>
> [drewes@cortex-0 root]$ svn --version
> Subversion Client, version 0.14.1 (dev build)
> compiled Aug 15 2002, 14:20:21
Still, the update bug on '.' that you're experiencing was fixed in the
last month. Update to today's code.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: permissions (and other) problems
Posted by Rich Drewes <dr...@interstice.com>.
cmpilato@collab.net wrote:
>Ben Collins-Sussman <su...@collab.net> writes:
>
>
>
>>>This is on subversion-r2092g.tar.gz.
>>>
>>>Anyone know what is going on here?
>>>
>>>
>>svn r2092! Yeesh, that's from last *May*!
>>
Sorry, I goofed, this was actually:
[drewes@cortex-0 root]$ svn --version
Subversion Client, version 0.14.1 (dev build)
compiled Aug 15 2002, 14:20:21
Rich
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: permissions (and other) problems
Posted by cm...@collab.net.
Ben Collins-Sussman <su...@collab.net> writes:
> > This is on subversion-r2092g.tar.gz.
> >
> > Anyone know what is going on here?
>
> svn r2092! Yeesh, that's from last *May*!
Worse than that. He's not running 2092 -- he's running the 2092g
tarball that exists *only* so folks can dump old repositories before
upgrading to a newer version of the codebase.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: permissions (and other) problems
Posted by Ben Collins-Sussman <su...@collab.net>.
Rich Drewes <dr...@interstice.com> writes:
> [drewes@cortex-0 newn48]$ svn update
> subversion/libsvn_wc/log.c:289: (apr_err=155009, src_err=0)
> svn: Problem running log
> svn: in directory .
> subversion/libsvn_wc/log.c:1155: (apr_err=155009, src_err=0)
> svn: start_handler: error processing command 'merge' in .
> subversion/libsvn_wc/log.c:436: (apr_err=155005, src_err=0)
> svn: Working copy not locked
> svn: svn_wc_merge() returned an unexpected error
> subversion/libsvn_wc/lock.c:363: (apr_err=155005, src_err=0)
> svn: directory not locked ()
>
> There are a bunch of lock files left hanging around. I don't think
> anything can save this working directory--not even manual deletion of
> lock files; user drewes will have to check out a new copy, reintegrate
> changes, and try again.
>
> This is on subversion-r2092g.tar.gz.
>
> Anyone know what is going on here?
svn r2092! Yeesh, that's from last *May*!
Isn't this an old bug, Philip? Some locking bug, or one that has to
do with running update in "."?
Rich -- try running 'svn cleanup'. And please upgrade to the latest
svn client.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: permissions (and other) problems
Posted by Philip Martin <ph...@codematters.co.uk>.
Rich Drewes <dr...@interstice.com> writes:
> [drewes@cortex-0 newn48]$ svn update
> subversion/libsvn_wc/log.c:289: (apr_err=155009, src_err=0)
> svn: Problem running log
> svn: in directory .
> subversion/libsvn_wc/log.c:1155: (apr_err=155009, src_err=0)
> svn: start_handler: error processing command 'merge' in .
> subversion/libsvn_wc/log.c:436: (apr_err=155005, src_err=0)
> svn: Working copy not locked
> svn: svn_wc_merge() returned an unexpected error
> subversion/libsvn_wc/lock.c:363: (apr_err=155005, src_err=0)
> svn: directory not locked ()
^^
I think this is fixed with rev 3113. You now say you are using
0.14.1 which is too old.
--
Philip Martin
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: permissions (and other) problems
Posted by Rich Drewes <dr...@interstice.com>.
Thanks for all the responses. I put in a wrapper script to set the umax
g+w before invoking svn, and that seems to solve some problems.
But here is another problem that seems to be independent of the
permission issue:
User drewes wants to commit the file Brain.cpp which has also been
updated by another user. drewes attempts to commit and gets this error:
[drewes@cortex-0 newn48]$ svn commit
Sending Brain.cpp
subversion/libsvn_client/commit.c:662: (apr_err=160028, src_err=0)
svn: Transaction is out of date
svn: Commit failed (details follow):
subversion/libsvn_ra_local/commit_editor.c:113: (apr_err=160028, src_err=0)
svn: out of date: `Brain.cpp' in txn `1g'
User drewes then attempts to update, to get the latest copy of Brain.cpp
so that it can be reconciled with his own copy and resubmitted:
[drewes@cortex-0 newn48]$ svn update
subversion/libsvn_wc/log.c:289: (apr_err=155009, src_err=0)
svn: Problem running log
svn: in directory .
subversion/libsvn_wc/log.c:1155: (apr_err=155009, src_err=0)
svn: start_handler: error processing command 'merge' in .
subversion/libsvn_wc/log.c:436: (apr_err=155005, src_err=0)
svn: Working copy not locked
svn: svn_wc_merge() returned an unexpected error
subversion/libsvn_wc/lock.c:363: (apr_err=155005, src_err=0)
svn: directory not locked ()
User drewes then tries to see what the hell is going on:
[drewes@cortex-0 newn48]$ svn status
_ L .
M Brain.cpp
? Brain.cpp.46080.00001.mine
? Brain.cpp.46084.00001.r49
? Brain.cpp.46088.00001.r48
? Brain.cpp.46160.00001.tmp
? Brain.cpp.46164.00001.tmp
? Brain.cpp.46164.00001.tmp.46152.00001.tmp
? Brain.cpp.46164.00001.tmp.46156.00001.tmp
_ L docs
_ L ear
_ L inputs
_ L parse
_ L scripts
There are a bunch of lock files left hanging around. I don't think
anything can save this working directory--not even manual deletion of
lock files; user drewes will have to check out a new copy, reintegrate
changes, and try again.
This is on subversion-r2092g.tar.gz.
Anyone know what is going on here?
Rich
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: permissions (and other) problems
Posted by br...@xbc.nu.
Quoting Rich Drewes <dr...@interstice.com>:
> Hello,
>
> 1. What is the right method of setting up permissions for using
> Subversion on a local filesystem with local users (Unix environment, of
> course)? I have had some serious problems with many different
> arrangments.
Yes, this is a problem we still haven't solved. I think the simplest thing would
be to do what CVS does -- namely, set umask to 002 internally, and tell users to
chmod g+rwXs on n all directories in the repo. Maybe "svnadmin create" could do
that for us?
Alternatively, this could be controlled by a server-side config option -- it's
not something that has to be communicated back to the client.
[snip]
> 2. More generally, is using Subversion with local users on the local
> filesystem (not remotely via http or whatever) supported very well, or are
> the remote access methods the only ones that work reliably? I have had
> lots of database lockup problems in the past and I'm not sure they are all
> going to go away now that I have the issue in #1 worked out with group
> write permission enabled in the users' umasks . . .
Apart from the restriction that the repository must be on a local disk (e.g.,
"local" access over NFS is impossible)? The permission problems may well have
been causing the lock-ups; the other p[robable cause interrupting svn (^C) while
it's fiddling with the repository (aargh, we really, really should find a way to
avoid fix that).
Brane
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org