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/11/19 12:24:19 UTC

Re: Serf crashes on fork

Stefan Fuhrmann <st...@wandisco.com> writes:

> As it turns out, your commit has only be the trigger but
> not the root cause.
>
> serf_trunk/allocator.c, serf_bucket_allocator_create(), line 147:
>
>     /* ### this implies buckets cannot cross a fork/exec. desirable?
>      *
>      * ### hmm. it probably also means that buckets cannot be AROUND
>      * ### during a fork/exec. the new process will try to clean them
>      * ### up and figure out there are unfreed blocks...
>      */
>     apr_pool_cleanup_register(pool, allocator,
>                               allocator_cleanup, allocator_cleanup);
>
> Since we fork() for hooks, we can't use hooks in ra_local
> while there is an open serf connection. Otherwise, we get
> into trouble with pool cleanups:

Does it ever make sense for the child process to run that handler?  Is
that to allow a parent process to allocate a serf connection and then
fork off a child process to use the connection?

-- 
Certified & Supported Apache Subversion Downloads:
http://www.wandisco.com/subversion/download

Re: Serf crashes on fork

Posted by Greg Stein <gs...@gmail.com>.
On Mon, Dec 3, 2012 at 7:30 AM, Stefan Fuhrmann
<st...@wandisco.com> wrote:
> On Thu, Nov 29, 2012 at 5:51 PM, C. Michael Pilato <cm...@collab.net>
> wrote:
>>
>> Perhaps this should be discussed on the serf dev list?  Or an issue filed
>> in
>> their tracker?
>
> Since I apprently can't post on serf-dev,

Sure you can. Subscribe first :-)

> I opened an issue for 1.8:
> http://subversion.tigris.org/issues/show_bug.cgi?id=4266

Marked as fixed, along with serf's corresponding issue 93.

it is fixed on serf trunk/ in r1714, and on branches/1.2.x (the
upcoming release) in r1716.

I imagine it should work just fine, but would be happy to hear any
testing reports.

Cheers,
-g

Re: Serf crashes on fork

Posted by Stefan Fuhrmann <st...@wandisco.com>.
On Thu, Nov 29, 2012 at 5:51 PM, C. Michael Pilato <cm...@collab.net>wrote:

> Perhaps this should be discussed on the serf dev list?  Or an issue filed
> in
> their tracker?
>

Since I apprently can't post on serf-dev, I opened an issue for 1.8:
http://subversion.tigris.org/issues/show_bug.cgi?id=4266

-- Stefan^2.

-- 
Certified & Supported Apache Subversion Downloads:
*

http://www.wandisco.com/subversion/download
*

Re: Serf crashes on fork

Posted by "C. Michael Pilato" <cm...@collab.net>.
Perhaps this should be discussed on the serf dev list?  Or an issue filed in
their tracker?


On 11/19/2012 10:09 AM, Stefan Fuhrmann wrote:
> On Mon, Nov 19, 2012 at 2:44 PM, Philip Martin <philip.martin@wandisco.com
> <ma...@wandisco.com>> wrote:
> 
>     Philip Martin <philip.martin@wandisco.com
>     <ma...@wandisco.com>> writes:
> 
>     > Stefan Fuhrmann <stefan.fuhrmann@wandisco.com
>     <ma...@wandisco.com>> writes:
>     >
>     >> On Mon, Nov 19, 2012 at 12:24 PM, Philip Martin
>     >> <philip.martin@wandisco.com <ma...@wandisco.com>>wrote:
>     >>
>     >>> Stefan Fuhrmann <stefan.fuhrmann@wandisco.com
>     <ma...@wandisco.com>> writes:
>     >>>
>     >>> > As it turns out, your commit has only be the trigger but
>     >>> > not the root cause.
>     >>> >
>     >>> > serf_trunk/allocator.c, serf_bucket_allocator_create(), line 147:
>     >>> >
>     >>> >     /* ### this implies buckets cannot cross a fork/exec. desirable?
>     >>> >      *
>     >>> >      * ### hmm. it probably also means that buckets cannot be AROUND
>     >>> >      * ### during a fork/exec. the new process will try to clean them
>     >>> >      * ### up and figure out there are unfreed blocks...
>     >>> >      */
>     >>> >     apr_pool_cleanup_register(pool, allocator,
>     >>> >                               allocator_cleanup, allocator_cleanup);
>     >>> >
>     >>> > Since we fork() for hooks, we can't use hooks in ra_local
>     >>> > while there is an open serf connection. Otherwise, we get
>     >>> > into trouble with pool cleanups:
>     >>>
>     >>> Does it ever make sense for the child process to run that handler?  Is
>     >>> that to allow a parent process to allocate a serf connection and then
>     >>> fork off a child process to use the connection?
> 
>     If the processes were behaving like that the child cleanup handlers
>     would not be involved.
> 
>     >>
>     >> From the comments in APR/threadproc/unix/proc.c,
>     >> it seems that apr_proc_create runs *all* pool cleanups
>     >> in the child process to clean up duplicated file handles
>     >> and such.
>     >
>     > apr_proc_create runs the child cleanup handlers.  Note that two handlers
>     > are passed to _register, one for the parent one for the child.  I'm
>     > asking why serf installs a child handler rather than passing
>     > apr_pool_cleanup_null.
> 
>     The cleanup handler is just releasing memory via apr_allocator_free. I
>     see no reason for it to be installed as a child cleanup handler.
> 
> 
> Simply patching serf fixes the problem for me.
> 
> -- Stefan^2.
> 
> [[[
> Index: buckets/allocator.c
> ===================================================================
> --- buckets/allocator.c    (revision 1685)
> +++ buckets/allocator.c    (working copy)
> @@ -151,7 +151,7 @@
>       * ### up and figure out there are unfreed blocks...
>       */
>      apr_pool_cleanup_register(pool, allocator,
> -                              allocator_cleanup, allocator_cleanup);
> +                              allocator_cleanup, apr_pool_cleanup_null);
>  
>      return allocator;
>  }
> ]]]
> 
> -- 
> Certified & Supported Apache Subversion Downloads:
> /
> 
> http://www.wandisco.com/subversion/download
> 
> /


-- 
C. Michael Pilato <cm...@collab.net>
CollabNet   <>   www.collab.net   <>   Enterprise Cloud Development


Re: Serf crashes on fork

Posted by Stefan Fuhrmann <st...@wandisco.com>.
On Mon, Nov 19, 2012 at 2:44 PM, Philip Martin
<ph...@wandisco.com>wrote:

> Philip Martin <ph...@wandisco.com> writes:
>
> > Stefan Fuhrmann <st...@wandisco.com> writes:
> >
> >> On Mon, Nov 19, 2012 at 12:24 PM, Philip Martin
> >> <ph...@wandisco.com>wrote:
> >>
> >>> Stefan Fuhrmann <st...@wandisco.com> writes:
> >>>
> >>> > As it turns out, your commit has only be the trigger but
> >>> > not the root cause.
> >>> >
> >>> > serf_trunk/allocator.c, serf_bucket_allocator_create(), line 147:
> >>> >
> >>> >     /* ### this implies buckets cannot cross a fork/exec. desirable?
> >>> >      *
> >>> >      * ### hmm. it probably also means that buckets cannot be AROUND
> >>> >      * ### during a fork/exec. the new process will try to clean them
> >>> >      * ### up and figure out there are unfreed blocks...
> >>> >      */
> >>> >     apr_pool_cleanup_register(pool, allocator,
> >>> >                               allocator_cleanup, allocator_cleanup);
> >>> >
> >>> > Since we fork() for hooks, we can't use hooks in ra_local
> >>> > while there is an open serf connection. Otherwise, we get
> >>> > into trouble with pool cleanups:
> >>>
> >>> Does it ever make sense for the child process to run that handler?  Is
> >>> that to allow a parent process to allocate a serf connection and then
> >>> fork off a child process to use the connection?
>
> If the processes were behaving like that the child cleanup handlers
> would not be involved.
>
> >>
> >> From the comments in APR/threadproc/unix/proc.c,
> >> it seems that apr_proc_create runs *all* pool cleanups
> >> in the child process to clean up duplicated file handles
> >> and such.
> >
> > apr_proc_create runs the child cleanup handlers.  Note that two handlers
> > are passed to _register, one for the parent one for the child.  I'm
> > asking why serf installs a child handler rather than passing
> > apr_pool_cleanup_null.
>
> The cleanup handler is just releasing memory via apr_allocator_free. I
> see no reason for it to be installed as a child cleanup handler.
>

Simply patching serf fixes the problem for me.

-- Stefan^2.

[[[
Index: buckets/allocator.c
===================================================================
--- buckets/allocator.c    (revision 1685)
+++ buckets/allocator.c    (working copy)
@@ -151,7 +151,7 @@
      * ### up and figure out there are unfreed blocks...
      */
     apr_pool_cleanup_register(pool, allocator,
-                              allocator_cleanup, allocator_cleanup);
+                              allocator_cleanup, apr_pool_cleanup_null);

     return allocator;
 }
]]]

-- 
Certified & Supported Apache Subversion Downloads:
*

http://www.wandisco.com/subversion/download
*

Re: Serf crashes on fork

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

> Stefan Fuhrmann <st...@wandisco.com> writes:
>
>> On Mon, Nov 19, 2012 at 12:24 PM, Philip Martin
>> <ph...@wandisco.com>wrote:
>>
>>> Stefan Fuhrmann <st...@wandisco.com> writes:
>>>
>>> > As it turns out, your commit has only be the trigger but
>>> > not the root cause.
>>> >
>>> > serf_trunk/allocator.c, serf_bucket_allocator_create(), line 147:
>>> >
>>> >     /* ### this implies buckets cannot cross a fork/exec. desirable?
>>> >      *
>>> >      * ### hmm. it probably also means that buckets cannot be AROUND
>>> >      * ### during a fork/exec. the new process will try to clean them
>>> >      * ### up and figure out there are unfreed blocks...
>>> >      */
>>> >     apr_pool_cleanup_register(pool, allocator,
>>> >                               allocator_cleanup, allocator_cleanup);
>>> >
>>> > Since we fork() for hooks, we can't use hooks in ra_local
>>> > while there is an open serf connection. Otherwise, we get
>>> > into trouble with pool cleanups:
>>>
>>> Does it ever make sense for the child process to run that handler?  Is
>>> that to allow a parent process to allocate a serf connection and then
>>> fork off a child process to use the connection?

If the processes were behaving like that the child cleanup handlers
would not be involved.

>>
>> From the comments in APR/threadproc/unix/proc.c,
>> it seems that apr_proc_create runs *all* pool cleanups
>> in the child process to clean up duplicated file handles
>> and such.
>
> apr_proc_create runs the child cleanup handlers.  Note that two handlers
> are passed to _register, one for the parent one for the child.  I'm
> asking why serf installs a child handler rather than passing
> apr_pool_cleanup_null.

The cleanup handler is just releasing memory via apr_allocator_free. I
see no reason for it to be installed as a child cleanup handler.

-- 
Certified & Supported Apache Subversion Downloads:
http://www.wandisco.com/subversion/download

Re: Serf crashes on fork

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

> On Mon, Nov 19, 2012 at 12:24 PM, Philip Martin
> <ph...@wandisco.com>wrote:
>
>> Stefan Fuhrmann <st...@wandisco.com> writes:
>>
>> > As it turns out, your commit has only be the trigger but
>> > not the root cause.
>> >
>> > serf_trunk/allocator.c, serf_bucket_allocator_create(), line 147:
>> >
>> >     /* ### this implies buckets cannot cross a fork/exec. desirable?
>> >      *
>> >      * ### hmm. it probably also means that buckets cannot be AROUND
>> >      * ### during a fork/exec. the new process will try to clean them
>> >      * ### up and figure out there are unfreed blocks...
>> >      */
>> >     apr_pool_cleanup_register(pool, allocator,
>> >                               allocator_cleanup, allocator_cleanup);
>> >
>> > Since we fork() for hooks, we can't use hooks in ra_local
>> > while there is an open serf connection. Otherwise, we get
>> > into trouble with pool cleanups:
>>
>> Does it ever make sense for the child process to run that handler?  Is
>> that to allow a parent process to allocate a serf connection and then
>> fork off a child process to use the connection?
>>
>
> From the comments in APR/threadproc/unix/proc.c,
> it seems that apr_proc_create runs *all* pool cleanups
> in the child process to clean up duplicated file handles
> and such.

apr_proc_create runs the child cleanup handlers.  Note that two handlers
are passed to _register, one for the parent one for the child.  I'm
asking why serf installs a child handler rather than passing
apr_pool_cleanup_null.

-- 
Certified & Supported Apache Subversion Downloads:
http://www.wandisco.com/subversion/download

Re: Serf crashes on fork

Posted by Stefan Fuhrmann <st...@wandisco.com>.
On Mon, Nov 19, 2012 at 12:24 PM, Philip Martin
<ph...@wandisco.com>wrote:

> Stefan Fuhrmann <st...@wandisco.com> writes:
>
> > As it turns out, your commit has only be the trigger but
> > not the root cause.
> >
> > serf_trunk/allocator.c, serf_bucket_allocator_create(), line 147:
> >
> >     /* ### this implies buckets cannot cross a fork/exec. desirable?
> >      *
> >      * ### hmm. it probably also means that buckets cannot be AROUND
> >      * ### during a fork/exec. the new process will try to clean them
> >      * ### up and figure out there are unfreed blocks...
> >      */
> >     apr_pool_cleanup_register(pool, allocator,
> >                               allocator_cleanup, allocator_cleanup);
> >
> > Since we fork() for hooks, we can't use hooks in ra_local
> > while there is an open serf connection. Otherwise, we get
> > into trouble with pool cleanups:
>
> Does it ever make sense for the child process to run that handler?  Is
> that to allow a parent process to allocate a serf connection and then
> fork off a child process to use the connection?
>

>From the comments in APR/threadproc/unix/proc.c,
it seems that apr_proc_create runs *all* pool cleanups
in the child process to clean up duplicated file handles
and such.

-- Stefan^2.

-- 
Certified & Supported Apache Subversion Downloads:
*

http://www.wandisco.com/subversion/download
*