RE: svn commit: r1134032 - /subversion/trunk/subversion/svnserve/main.c

[Bert Huijben]

Pre pool cleanup is not in Apr 0.9.x if I remember correctly.

Bert Huijben (Cell phone) From: Daniel Shahaf
Sent: donderdag 7 juli 2011 18:59
To: dev@subversion.apache.org
Cc: comm...@subversion.apache.org; Alec Kloss
Subject: Re: svn commit: r1134032
- /subversion/trunk/subversion/svnserve/main.c
s...@apache.org wrote on Thu, Jun 09, 2011 at 18:39:44 -0000:
> Author: stsp
> Date: Thu Jun  9 18:39:43 2011
> New Revision: 1134032
>
> URL: http://svn.apache.org/viewvc?rev=1134032&view=rev
> Log:
> Fix issue #3664, "SASL support in inetd mode caused SIGSEGV during shutdown".
>
> Make sure svnserve pool cleanup handlers that call sasl_dispose()
> and sasl_done() are run in the right order.
>
> Patch by: Alec Kloss <alec.kloss at oracle.com>
>
> * subversion/svnserve/main.c
>   (main): Create a subpool to handle the connection in inetd mode as well.
>
> Modified:
>     subversion/trunk/subversion/svnserve/main.c
>
> Modified: subversion/trunk/subversion/svnserve/main.c
> URL: 
> http://svn.apache.org/viewvc/subversion/trunk/subversion/svnserve/main.c?rev=1134032&r1=1134031&r2=1134032&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/svnserve/main.c (original)
> +++ subversion/trunk/subversion/svnserve/main.c Thu Jun  9 18:39:43 2011
> @@ -707,9 +707,14 @@ int main(int argc, const char *argv[])
>            return svn_cmdline_handle_exit_error(err, pool, "svnserve: ");
>          }
>
> +      /* Use a subpool for the connection to ensure that if SASL is used
> +       * the pool cleanup handlers that call sasl_dispose() (connection_pool)
> +       * and sasl_done() (pool) are run in the right order. See issue #3664. 
> */
> +      connection_pool = svn_pool_create(pool);
>        conn = svn_ra_svn_create_conn2(NULL, in_file, out_file,
> -                                     params.compression_level, pool);
> -      svn_error_clear(serve(conn, &params, pool));
> +                                     params.compression_level,
> +                                     connection_pool);
> +      svn_error_clear(serve(conn, &params, connection_pool));
>        exit(0);
>      }
>
>
>

Instead of creating a new pool, could we just reorder the cleanups here?

[[[
Index: subversion/svnserve/main.c
===================================================================
--- subversion/svnserve/main.c  (revision 1143898)
+++ subversion/svnserve/main.c  (working copy)
@@ -710,11 +710,10 @@ int main(int argc, const char *argv[])
       /* Use a subpool for the connection to ensure that if SASL is used
        * the pool cleanup handlers that call sasl_dispose() (connection_pool)
        * and sasl_done() (pool) are run in the right order. See issue #3664. */
-      connection_pool = svn_pool_create(pool);
       conn = svn_ra_svn_create_conn2(NULL, in_file, out_file,
                                      params.compression_level,
-                                     connection_pool);
-      svn_error_clear(serve(conn, &params, connection_pool));
+                                     pool);
+      svn_error_clear(serve(conn, &params, pool));
       exit(0);
     }

Index: subversion/svnserve/cyrus_auth.c
===================================================================
--- subversion/svnserve/cyrus_auth.c    (revision 1143898)
+++ subversion/svnserve/cyrus_auth.c    (working copy)
@@ -282,8 +282,7 @@ svn_error_t *cyrus_auth_request(svn_ra_svn_conn_t
     }

   /* Make sure the context is always destroyed. */
-  apr_pool_cleanup_register(b->pool, sasl_ctx, sasl_dispose_cb,
-                            apr_pool_cleanup_null);
+  apr_pool_pre_cleanup_register(b->pool, sasl_ctx, sasl_dispose_cb);

   /* Initialize security properties. */
   svn_ra_svn__default_secprops(&secprops);
]]]

I haven't tested this since I don't have an SASL setup handy :-(.