You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Philip Martin <ph...@wandisco.com> on 2012/04/23 18:26:24 UTC

neon pool problem causing buildbot fail

The openbsd buildbot failed:

http://ci.apache.org/builders/bb-openbsd/builds/1139

like this:

tree_conflicts_tests-23 is failing when importing a file that

2012-04-22 16:55:49 [WARNING] subversion/svn/import-cmd.c:127: (apr_err=2)
subversion/libsvn_client/commit.c:971: (apr_err=2)
subversion/libsvn_client/commit.c:971: (apr_err=2)
subversion/libsvn_client/commit.c:631: (apr_err=2)
subversion/libsvn_subr/io.c:2425: (apr_err=2)
subversion/libsvn_subr/io.c:3406: (apr_err=2)
svn: E000002: Can't stat '/home/buildslave16/slave16/bb-openbsd/build/subversion/tests/cmdline/svn-test-work/working_copies/tree_conflict_tests-23/A/foo': No such file or directory

2012-04-22 16:55:49 [WARNING] CMD: /home/buildslave16/slave16/bb-openbsd/build/subversion/svn/svn import -m 'File '\''/home/buildslave16/slave16/bb-openbsd/svn-trunk/subversion/tests/cmdline/tree_conflict_tests.py'\'', line 1221, in actual_only_node_behaviour' svn-test-work/working_copies/tree_conflict_tests-23/A/foo http://localhost:8081/svn-test-work/repositories/tree_conflict_tests-23/foo_imported --config-dir /home/buildslave16/slave16/bb-openbsd/build/subversion/tests/cmdline/svn-test-work/local_tmp/config --password rayjandom --no-auth-cache --username jrandom terminated by signal 11
2012-04-22 16:55:49 [WARNING] CWD: /home/buildslave16/slave16/bb-openbsd/build/subversion/tests/cmdline
2012-04-22 16:55:49 [WARNING] EXCEPTION: SVNProcessTerminatedBySignal
Traceback (most recent call last):
  File "/home/buildslave16/slave16/bb-openbsd/svn-trunk/subversion/tests/cmdline/svntest/main.py", line 1332, in run
    rc = self.pred.run(sandbox)
  File "/home/buildslave16/slave16/bb-openbsd/svn-trunk/subversion/tests/cmdline/svntest/testcase.py", line 176, in run
    return self.func(sandbox)
  File "/home/buildslave16/slave16/bb-openbsd/svn-trunk/subversion/tests/cmdline/tree_conflict_tests.py", line 1222, in actual_only_node_behaviour
    foo_path, sbox.repo_url + '/foo_imported')

The import is expected to fail with "Can't stat" but it appears that the
client is crashing when exiting.  On my Linux box I'm seeing valgrind
errors introduced by r1327801:

==19213== Invalid read of size 8
==19213==    at 0x60B4D86: ??? (in /usr/lib/libneon.so.27.2.3)
==19213==    by 0x60BEBBC: ne_decompress_destroy (in /usr/lib/libneon.so.27.2.3)
==19213==    by 0x70E7EF0: compressed_body_reader_cleanup (util.c:438)
==19213==    by 0x5E8D46A: run_cleanups (in /usr/lib/libapr-1.so.0.4.2)
==19213==    by 0x5E8C338: apr_pool_destroy (in /usr/lib/libapr-1.so.0.4.2)
==19213==    by 0x5E8C31B: apr_pool_destroy (in /usr/lib/libapr-1.so.0.4.2)
==19213==    by 0x5E8C31B: apr_pool_destroy (in /usr/lib/libapr-1.so.0.4.2)
==19213==    by 0x5E8D844: apr_pool_destroy_debug (in /usr/lib/libapr-1.so.0.4.2)
==19213==    by 0x417095: main (main.c:2728)
==19213==  Address 0xb04da58 is 232 bytes inside a block of size 888 free'd
==19213==    at 0x4C240FD: free (vg_replace_malloc.c:366)
==19213==    by 0x70E3F8B: cleanup_session (session.c:63)
==19213==    by 0x5E8D46A: run_cleanups (in /usr/lib/libapr-1.so.0.4.2)
==19213==    by 0x5E8C338: apr_pool_destroy (in /usr/lib/libapr-1.so.0.4.2)
==19213==    by 0x5E8C31B: apr_pool_destroy (in /usr/lib/libapr-1.so.0.4.2)
==19213==    by 0x5E8D844: apr_pool_destroy_debug (in /usr/lib/libapr-1.so.0.4.2)
==19213==    by 0x417095: main (main.c:2728)

I can fix the valgrind errors with this patch:

Index: subversion/libsvn_client/commit.c
===================================================================
--- subversion/libsvn_client/commit.c	(revision 1327801)
+++ subversion/libsvn_client/commit.c	(working copy)
@@ -782,7 +782,7 @@
 
   SVN_ERR(svn_client__open_ra_session_internal(&ra_session, NULL, url, NULL,
                                                NULL, FALSE, TRUE, ctx,
-                                               scratch_pool));
+                                               iterpool));
 
   /* Figure out all the path components we need to create just to have
      a place to stick our imported tree. */
@@ -790,8 +790,6 @@
                             iterpool));
   while (kind == svn_node_none)
     {
-      svn_pool_clear(iterpool);
-
       svn_uri_split(&temp, &dir, url, scratch_pool);
       APR_ARRAY_PUSH(new_entries, const char *) = dir;
       url = temp;

I'm not sure exactly how the pool management is supposed to work here.

-- 
uberSVN: Apache Subversion Made Easy
http://www.uberSVN.com

Re: neon pool problem causing buildbot fail

Posted by Philip Martin <ph...@codematters.co.uk>.
Philip Martin <ph...@wandisco.com> writes:

> Philip Martin <ph...@wandisco.com> writes:
>
>> I can fix the valgrind errors with this patch:
>>
>> Index: subversion/libsvn_client/commit.c
>> ===================================================================
>> --- subversion/libsvn_client/commit.c	(revision 1327801)
>> +++ subversion/libsvn_client/commit.c	(working copy)
>> @@ -782,7 +782,7 @@
>>  
>>    SVN_ERR(svn_client__open_ra_session_internal(&ra_session, NULL, url, NULL,
>>                                                 NULL, FALSE, TRUE, ctx,
>> -                                               scratch_pool));
>> +                                               iterpool));
>>  
>>    /* Figure out all the path components we need to create just to have
>>       a place to stick our imported tree. */
>> @@ -790,8 +790,6 @@
>>                              iterpool));
>>    while (kind == svn_node_none)
>>      {
>> -      svn_pool_clear(iterpool);
>> -
>>        svn_uri_split(&temp, &dir, url, scratch_pool);
>>        APR_ARRAY_PUSH(new_entries, const char *) = dir;
>>        url = temp;
>>
>> I'm not sure exactly how the pool management is supposed to work here.
>
> That patch is against r1327801 but it also applies to HEAD.  This
> patch to HEAD also fixes it:
>
>
> Index: subversion/libsvn_client/commit.c
> ===================================================================
> --- subversion/libsvn_client/commit.c	(revision 1329331)
> +++ subversion/libsvn_client/commit.c	(working copy)
> @@ -968,6 +968,7 @@
>                      ctx, iterpool)))
>      {
>        svn_error_clear(editor->abort_edit(edit_baton, iterpool));
> +      svn_pool_destroy(iterpool);
>        return svn_error_trace(err);
>      }
>  
> as does this:
>
>
> Index: subversion/libsvn_client/commit.c
> ===================================================================
> --- subversion/libsvn_client/commit.c	(revision 1329331)
> +++ subversion/libsvn_client/commit.c	(working copy)
> @@ -965,7 +965,7 @@
>                      depth, excludes, no_ignore,
>                      ignore_unknown_node_types,
>                      filter_callback, filter_baton,
> -                    ctx, iterpool)))
> +                    ctx, scratch_pool)))
>      {
>        svn_error_clear(editor->abort_edit(edit_baton, iterpool));
>        return svn_error_trace(err);

Although these "work" I think they are simply covering up the real
problem.  In libsvn_ra_neon/commit.c:commit_open_root there is a call to
svn_ra_neon__request_create but no call to svn_ra_neon__request_destroy.
That missing destroy call means that the cleanup handler for the
create-txn request runs much later than those of all the other requests.

-- 
Philip

Re: neon pool problem causing buildbot fail

Posted by Philip Martin <ph...@wandisco.com>.
Philip Martin <ph...@wandisco.com> writes:

> I can fix the valgrind errors with this patch:
>
> Index: subversion/libsvn_client/commit.c
> ===================================================================
> --- subversion/libsvn_client/commit.c	(revision 1327801)
> +++ subversion/libsvn_client/commit.c	(working copy)
> @@ -782,7 +782,7 @@
>  
>    SVN_ERR(svn_client__open_ra_session_internal(&ra_session, NULL, url, NULL,
>                                                 NULL, FALSE, TRUE, ctx,
> -                                               scratch_pool));
> +                                               iterpool));
>  
>    /* Figure out all the path components we need to create just to have
>       a place to stick our imported tree. */
> @@ -790,8 +790,6 @@
>                              iterpool));
>    while (kind == svn_node_none)
>      {
> -      svn_pool_clear(iterpool);
> -
>        svn_uri_split(&temp, &dir, url, scratch_pool);
>        APR_ARRAY_PUSH(new_entries, const char *) = dir;
>        url = temp;
>
> I'm not sure exactly how the pool management is supposed to work here.

That patch is against r1327801 but it also applies to HEAD.  This
patch to HEAD also fixes it:


Index: subversion/libsvn_client/commit.c
===================================================================
--- subversion/libsvn_client/commit.c	(revision 1329331)
+++ subversion/libsvn_client/commit.c	(working copy)
@@ -968,6 +968,7 @@
                     ctx, iterpool)))
     {
       svn_error_clear(editor->abort_edit(edit_baton, iterpool));
+      svn_pool_destroy(iterpool);
       return svn_error_trace(err);
     }
 
as does this:


Index: subversion/libsvn_client/commit.c
===================================================================
--- subversion/libsvn_client/commit.c	(revision 1329331)
+++ subversion/libsvn_client/commit.c	(working copy)
@@ -965,7 +965,7 @@
                     depth, excludes, no_ignore,
                     ignore_unknown_node_types,
                     filter_callback, filter_baton,
-                    ctx, iterpool)))
+                    ctx, scratch_pool)))
     {
       svn_error_clear(editor->abort_edit(edit_baton, iterpool));
       return svn_error_trace(err);


-- 
uberSVN: Apache Subversion Made Easy
http://www.uberSVN.com