httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Brad Nicholes" <BNICHO...@novell.com>
Subject Re: svn commit: r646582 - /httpd/httpd/trunk/modules/ldap/util_ldap.c
Date Thu, 10 Apr 2008 23:34:36 GMT
>>> On 4/10/2008 at 2:00 PM, in message <47FE71F2.8030004@apache.org>,
Ruediger
Pluem <rpluem@apache.org> wrote:
> On 10.04.2008 18:11, Brad Nicholes wrote:
>>>>> On 4/10/2008 at 12:12 AM, in message
<47FDAFE4.4070806@apache.org>,
>> Ruediger
>> Pluem <rpluem@apache.org> wrote:
>>> On 10.04.2008 00:49, bnicholes@apache.org wrote:
>>>> Author: bnicholes Date: Wed Apr  9 15:49:31 2008 New Revision:
>> 646582
>>>> URL: http://svn.apache.org/viewvc?rev=646582&view=rev Log: Move
the
>>>> initialization of rebind to the post_config handler so that it is
>> done 
>>> during
>>>> the actual module load stage rather than the preload stage.  If
done
>> during
>>>> the preload stage, the pool passed into the initialization
function
>> will be
>>>> cleared and all allocations will be freed.
>>> But isn't it the pconf pool in both cases? Currently I cannot
follow
>> this 
>>> argument. Mind to explain in more detail?
>>>
>>> Regards
>>>
>>> RĂ¼diger
>> 
>> What is happening is that when httpd is invoked, the first thing it
>> does is preload all of the modules to make sure that everything
works. 
>> The preload of the modules also calls the util_ldap_create_config()
>> which subsequently calls apr_ldap_rebind_init().  In
>> apr_ldap_rebind_init() some allocations are done.  Actually a
thread
>> mutex is created against the pool that was passed in.  In addition,
the
>> clean up of the mutex was tied to the pool and the rebind init
function
>> also assigned to a global static variable, the mutex handle.  Then
when
>> the preload stage is complete, it clears the memory pool which also
>> causes the mutex to be destroy.  However the mutex handle, which is
now
>> bogus, is still assigned to the global static variable.  Finally
the
>> real module loading stage occurs and util_ldap_create_config() and
>> apr_ldap_rebind_init() are called again.  Only this time the
>> apr_ldap_rebind_init() sees that the mutex global static variable is
no
>> longer NULL so it doesn't bother to recreate the mutex leaving a
bogus
>> value in the static variable.  When an ldap authentication occurs
later
>> and tries to use the bogus mutex handle, at least on NetWare, the
code
>> segfaults. 
>> 
>> The change was simply moving the init of the rebind functionality to
a
>> location in the code where everything else is initialized and is
also
>> protected so that the initialization only happens during the real
module
>> load stage rather than the preload stage.
> 
> Thanks for explaining. I now understand why it is bad to call 
> apr_ldap_rebind_init twice, but I still do not get how your change
helps 
> avoiding this as the post_config hook is also called twice and the
pconf
> pool that is used for apr_ldap_rebind_init is cleaned up in between.
> Sorry for being persistent on this, but care to explain where my
thoughts
> are wrong here?
> In other parts of the code these situations are normally solved by
something
> like if (staticvar++) {} with staticvar being an static global int 
> initialized 
> with 0.
> 

It is protected by the code at the beginning of util_ldap_post_config()
that calls apr_pool_userdata_get() and checks a user data tag.  If the
tag is empty then this is the first time that the function was called. 
It then puts something in the tag and returns.  The next time it reads
the user data tag, it finds what it put there the first time.  Now it
knows that this is the second time and goes ahead with initializations. 


This is actually the preferred way to do it rather than using a static
variable.  Static global variables only work on platforms that have
process separation for data areas.  NetWare *isn't* one of those
platforms.  A global variable is global to everything running on the
box.  That was why I had to make the other NetWare specific changes in
ldap_rebind.c.  Static globals are bad. :(

Brad

Mime
View raw message