The attached patch adds cancellation support to svnadmin and svnlook.
Pressing ctrl-c when using these programs will wedge the repository
without this patch.
The patch also ignores SIGPIPE for svnadmin, svnlook and svn.
Redirecting the output through a pipe to e.g. more will wedge the
repository without this patch if more is terminated. Running a command
on a remote machine will cause the same problem if the connection is
terminated.
I doubt that many of our users understand that pressing ctrl-c during
"svnadmin verify" or running "svnlook cat" through more and quitting
more can cause the repository to wedge. It's a sneaky problem.
We do have a lot more negative user experience that we deserve because
repositories get wedged much too easily. The most common problem is
likely the permission problem, but the problems that the attached patch
fixes are also common.
This patch does not fix all problems, but many of them. I'll bring up
the rest that I know of in separate emails.
Unless I get a lot of negative feedback I plan to put this in an issue
and make it a 1.0 vote. I wanted to post it here first to make it
easier for you all to look at the patch and give comments.
/Tobias
[[[
!!! Contains API changes !!!
Add cancellation support to the svn_repos dump/load/parse functions,
and add lots of cancellation points in the svnlook code. Add Ctrl-C
handling to svnadmin and svnlook using the new cancellation
support. Ignore SIGPIPE in svnadmin, svnlook and the cmdline client to
prevent wedging the repos when piping the output or on network errors.
* subversion/svnadmin/main.c
(cancelled): New global variable to handle cancellation.
(sig_int): New handler for SIGINT.
(check_cancel): New cancellation function.
(subcommand_deltify): Check for cancellation.
(subcommand_dump): Pass check_cancel to svn_repos_dump_fs.
(subcommand_load): Pass check_cancel to svn_repos_load_fs.
(subcommand_verify): Pass check_cancel to svn_repos_dump_fs.
(main): Install the SIGINT handler and ignore SIGPIPE.
* subversion/include/svn_repos.h
(svn_repos_dump_fs): Add cancellation function and baton arguments.
(svn_repos_load_fs): Add cancellation function and baton arguments.
(svn_repos_parse_dumpstream): Add cancellation function and baton
arguments.
* subversion/svnlook/main.c
(cancelled): New global variable to handle cancellation.
(sig_int): New handler for SIGINT.
(check_cancel): New cancellation function.
(print_dirs_changed_tree, print_changed_tree, dump_contents,
display_prop_diffs, print_diff_tree, print_tree, do_cat,
print_history, do_plist): Check for cancellation.
(main): Install the SIGINT handler and ignore SIGPIPE.
* subversion/clients/cmdline/main.c
(main): Ignore SIGPIPE.
* subversion/svndumpfilter/main.c
(subcommand_exclude, subcommand_include): Add NULL cancellation function
and baton for calls to svn_repos_parse_dumpstream.
* subversion/libsvn_repos/load.c
(svn_repos_load_fs): Add cancellation function and baton arguments
and pass them on to svn_repos_parse_dumpstream.
(svn_repos_parse_dumpstream): Add cancellation function and baton
arguments. Check for cancellation.
* subversion/libsvn_repos/dump.c
(svn_repos_dump_fs): Add cancellation function and baton arguments.
Check for cancellation.
]]]
Index: subversion/svnadmin/main.c
===================================================================
--- subversion/svnadmin/main.c (revision 8163)
+++ subversion/svnadmin/main.c (working copy)
@@ -18,6 +18,7 @@
#include <apr_file_io.h>
+#include <apr_signal.h>
#include "svn_pools.h"
#include "svn_cmdline.h"
@@ -34,9 +35,28 @@
/*** Code. ***/
-/* Helper to open stdio streams */
+/* A flag to see if we've been cancelled by the client or not. */
+static volatile sig_atomic_t cancelled = FALSE;
+/* A signal handler to support cancellation. */
+static void
+sig_int (int unused)
+{
+ cancelled = TRUE;
+}
+
+/* Our cancellation callback. */
static svn_error_t *
+check_cancel (void *baton)
+{
+ if (cancelled)
+ return svn_error_create (SVN_ERR_CANCELLED, NULL, "caught SIGINT");
+ else
+ return SVN_NO_ERROR;
+}
+
+/* Helper to open stdio streams */
+static svn_error_t *
create_stdio_stream (svn_stream_t **stream,
APR_DECLARE(apr_status_t) open_fn (apr_file_t **,
apr_pool_t *),
@@ -371,6 +391,7 @@
for (revision = start; revision <= end; revision++)
{
svn_pool_clear (subpool);
+ SVN_ERR (check_cancel (NULL));
if (! opt_state->quiet)
printf ("Deltifying revision %" SVN_REVNUM_T_FMT "...", revision);
SVN_ERR (svn_fs_deltify_revision (fs, revision, subpool));
@@ -444,7 +465,8 @@
apr_file_open_stderr, pool));
SVN_ERR (svn_repos_dump_fs (repos, stdout_stream, stderr_stream,
- lower, upper, opt_state->incremental, pool));
+ lower, upper, opt_state->incremental,
+ check_cancel, NULL, pool));
return SVN_NO_ERROR;
}
@@ -492,7 +514,7 @@
SVN_ERR (svn_repos_load_fs (repos, stdin_stream, stdout_stream,
opt_state->uuid_action, opt_state->parent_dir,
- pool));
+ check_cancel, NULL, pool));
return SVN_NO_ERROR;
}
@@ -733,7 +755,7 @@
SVN_ERR (svn_fs_youngest_rev (&youngest, svn_repos_fs (repos), pool));
SVN_ERR (create_stdio_stream (&stderr_stream, apr_file_open_stderr, pool));
SVN_ERR (svn_repos_dump_fs (repos, NULL, stderr_stream,
- 0, youngest, FALSE, pool));
+ 0, youngest, FALSE, check_cancel, NULL, pool));
return SVN_NO_ERROR;
}
@@ -1011,6 +1033,14 @@
}
}
+ /* Set up our cancellation support. */
+ apr_signal (SIGINT, sig_int);
+
+#ifdef SIGPIPE
+ /* Disable SIGPIPE generation for the platforms that have it. */
+ apr_signal(SIGPIPE, SIG_IGN);
+#endif
+
/* Run the subcommand. */
err = (*subcommand->cmd_func) (os, &opt_state, pool);
if (err)
Index: subversion/include/svn_repos.h
===================================================================
--- subversion/include/svn_repos.h (revision 8163)
+++ subversion/include/svn_repos.h (working copy)
@@ -860,6 +860,10 @@
* If @a incremental is @c TRUE, the first revision dumped will be a diff
* against the previous revision (usually it looks like a full dump of
* the tree).
+ *
+ * If @a cancel_func is not @c NULL, it is called periodically with
+ * @a cancel_baton as argument to see if the client wishes to cancel
+ * the dump.
*/
svn_error_t *svn_repos_dump_fs (svn_repos_t *repos,
svn_stream_t *dumpstream,
@@ -867,6 +871,8 @@
svn_revnum_t start_rev,
svn_revnum_t end_rev,
svn_boolean_t incremental,
+ svn_cancel_func_t cancel_func,
+ void *cancel_baton,
apr_pool_t *pool);
@@ -889,12 +895,18 @@
*
* If the dumpstream contains no UUID, then @a uuid_action is
* ignored and the repository UUID is not touched.
+ *
+ * If @a cancel_func is not @c NULL, it is called periodically with
+ * @a cancel_baton as argument to see if the client wishes to cancel
+ * the dump.
*/
svn_error_t *svn_repos_load_fs (svn_repos_t *repos,
svn_stream_t *dumpstream,
svn_stream_t *feedback_stream,
enum svn_repos_load_uuid uuid_action,
const char *parent_dir,
+ svn_cancel_func_t cancel_func,
+ void *cancel_baton,
apr_pool_t *pool);
@@ -974,6 +986,10 @@
/** Read and parse dumpfile-formatted @a stream, calling callbacks in
* @a parse_fns/@a parse_baton, and using @a pool for allocations.
*
+ * If @a cancel_func is not @c NULL, it is called periodically with
+ * @a cancel_baton as argument to see if the client wishes to cancel
+ * the dump.
+ *
* This parser has built-in knowledge of the dumpfile format, but only
* in a general sense:
*
@@ -993,6 +1009,8 @@
svn_repos_parse_dumpstream (svn_stream_t *stream,
const svn_repos_parser_fns_t *parse_fns,
void *parse_baton,
+ svn_cancel_func_t cancel_func,
+ void *cancel_baton,
apr_pool_t *pool);
Index: subversion/svnlook/main.c
===================================================================
--- subversion/svnlook/main.c (revision 8163)
+++ subversion/svnlook/main.c (working copy)
@@ -23,6 +23,7 @@
#include <apr_time.h>
#include <apr_thread_proc.h>
#include <apr_file_io.h>
+#include <apr_signal.h>
#define APR_WANT_STDIO
#define APR_WANT_STRFUNC
@@ -233,10 +234,30 @@
} svnlook_ctxt_t;
+/* A flag to see if we've been cancelled by the client or not. */
+static volatile sig_atomic_t cancelled = FALSE;
/*** Helper functions. ***/
+
+/* A signal handler to support cancellation. */
+static void
+sig_int (int unused)
+{
+ cancelled = TRUE;
+}
+
+/* Our cancellation callback. */
static svn_error_t *
+check_cancel (void *baton)
+{
+ if (cancelled)
+ return svn_error_create (SVN_ERR_CANCELLED, NULL, "caught SIGINT");
+ else
+ return SVN_NO_ERROR;
+}
+
+static svn_error_t *
get_property (svn_string_t **prop_value /* native */,
svn_boolean_t need_translation,
svnlook_ctxt_t *c,
@@ -336,6 +357,8 @@
int print_me = 0;
const char *full_path;
+ SVN_ERR (check_cancel (NULL));
+
if (! node)
return SVN_NO_ERROR;
@@ -411,6 +434,8 @@
char status[3] = "_ ";
int print_me = 1;
+ SVN_ERR (check_cancel (NULL));
+
if (! node)
return SVN_NO_ERROR;
@@ -536,6 +561,7 @@
/* Now, route that data into our temporary file. */
while (1)
{
+ SVN_ERR (check_cancel (NULL));
len = sizeof (buffer);
SVN_ERR (svn_stream_read (stream, buffer, &len));
len2 = len;
@@ -702,6 +728,8 @@
const svn_string_t *orig_value;
const svn_prop_t *pc = &APR_ARRAY_IDX (prop_diffs, i, svn_prop_t);
+ SVN_ERR (check_cancel (NULL));
+
if (orig_props)
orig_value = apr_hash_get (orig_props, pc->name, APR_HASH_KEY_STRING);
else
@@ -762,6 +790,8 @@
svn_boolean_t is_copy = FALSE;
svn_boolean_t binary = FALSE;
+ SVN_ERR (check_cancel (NULL));
+
if (! node)
return SVN_NO_ERROR;
@@ -981,6 +1011,8 @@
apr_hash_t *entries;
apr_hash_index_t *hi;
+ SVN_ERR (check_cancel (NULL));
+
/* Print the indentation. */
for (i = 0; i < indentation; i++)
{
@@ -1181,6 +1213,7 @@
SVN_ERR (svn_fs_file_contents (&fstream, root, path, pool));
SVN_ERR (svn_stream_for_stdout (&stdout_stream, pool));
do {
+ SVN_ERR (check_cancel (NULL));
SVN_ERR (svn_stream_read (fstream, buf, &len));
SVN_ERR (svn_stream_write (stdout_stream, buf, &len));
} while (len == BUFSIZ);
@@ -1275,6 +1308,8 @@
{
struct print_history_baton *phb = baton;
+ SVN_ERR (check_cancel (NULL));
+
if (phb->show_ids)
{
const svn_fs_id_t *node_id;
@@ -1396,6 +1431,8 @@
const char *pname;
svn_string_t *propval;
+ SVN_ERR (check_cancel (NULL));
+
apr_hash_this (hi, &key, NULL, &val);
pname = key;
propval = val;
@@ -1922,6 +1959,14 @@
}
}
+ /* Set up our cancellation support. */
+ apr_signal (SIGINT, sig_int);
+
+#ifdef SIGPIPE
+ /* Disable SIGPIPE generation for the platforms that have it. */
+ apr_signal(SIGPIPE, SIG_IGN);
+#endif
+
/* Run the subcommand. */
err = (*subcommand->cmd_func) (os, &opt_state, pool);
if (err)
Index: subversion/clients/cmdline/main.c
===================================================================
--- subversion/clients/cmdline/main.c (revision 8163)
+++ subversion/clients/cmdline/main.c (working copy)
@@ -1236,6 +1236,11 @@
(void *) "");
}
+#ifdef SIGPIPE
+ /* Disable SIGPIPE generation for the platforms that have it. */
+ apr_signal(SIGPIPE, SIG_IGN);
+#endif
+
/* And now we finally run the subcommand. */
err = (*subcommand->cmd_func) (os, &command_baton, pool);
if (err)
Index: subversion/svndumpfilter/main.c
===================================================================
--- subversion/svndumpfilter/main.c (revision 8163)
+++ subversion/svndumpfilter/main.c (working copy)
@@ -854,7 +854,8 @@
SVN_ERR (parse_baton_initialize (&pb, opt_state, TRUE, pool));
SVN_ERR (svn_repos_parse_dumpstream (pb->in_stream,
- &filtering_vtable, pb, pool));
+ &filtering_vtable, pb,
+ NULL, NULL, pool));
fprintf (stderr, "\n Dropped %d revisions, %d nodes\n",
pb->rev_drop_count, pb->node_drop_count);
@@ -888,7 +889,8 @@
SVN_ERR (parse_baton_initialize (&pb, opt_state, FALSE, pool));
SVN_ERR (svn_repos_parse_dumpstream (pb->in_stream,
- &filtering_vtable, pb, pool));
+ &filtering_vtable, pb,
+ NULL, NULL, pool));
fprintf (stderr, "\n Dropped %d revisions, %d nodes\n",
pb->rev_drop_count, pb->node_drop_count);
Index: subversion/libsvn_repos/load.c
===================================================================
--- subversion/libsvn_repos/load.c (revision 8163)
+++ subversion/libsvn_repos/load.c (working copy)
@@ -387,6 +387,8 @@
svn_repos_parse_dumpstream (svn_stream_t *stream,
const svn_repos_parser_fns_t *parse_fns,
void *parse_baton,
+ svn_cancel_func_t cancel_func,
+ void *cancel_baton,
apr_pool_t *pool)
{
svn_boolean_t eof;
@@ -432,6 +434,10 @@
/* Clear our per-line pool. */
svn_pool_clear (linepool);
+ /* Check for cancellation. */
+ if (cancel_func)
+ SVN_ERR (cancel_func (cancel_baton));
+
/* Keep reading blank lines until we discover a new record, or until
the stream runs out. */
SVN_ERR (svn_stream_readline (stream, &linebuf, "\n", &eof, linepool));
@@ -1031,6 +1037,8 @@
svn_stream_t *feedback_stream,
enum svn_repos_load_uuid uuid_action,
const char *parent_dir,
+ svn_cancel_func_t cancel_func,
+ void *cancel_baton,
apr_pool_t *pool)
{
const svn_repos_parser_fns_t *parser;
@@ -1046,7 +1054,8 @@
parent_dir,
pool));
- SVN_ERR (svn_repos_parse_dumpstream (dumpstream, parser, parse_baton, pool));
+ SVN_ERR (svn_repos_parse_dumpstream (dumpstream, parser, parse_baton,
+ cancel_func, cancel_baton, pool));
return SVN_NO_ERROR;
}
Index: subversion/libsvn_repos/dump.c
===================================================================
--- subversion/libsvn_repos/dump.c (revision 8163)
+++ subversion/libsvn_repos/dump.c (working copy)
@@ -855,6 +855,8 @@
svn_revnum_t start_rev,
svn_revnum_t end_rev,
svn_boolean_t incremental,
+ svn_cancel_func_t cancel_func,
+ void *cancel_baton,
apr_pool_t *pool)
{
const svn_delta_editor_t *dump_editor;
@@ -910,6 +912,10 @@
svn_revnum_t from_rev, to_rev;
svn_fs_root_t *to_root;
+ /* Check for cancellation. */
+ if (cancel_func)
+ SVN_ERR (cancel_func (cancel_baton));
+
/* Special-case the initial revision dump: it needs to contain
*all* nodes, because it's the foundation of all future
revisions in the dumpfile. */
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Tue Jan 6 12:16:05 2004