You are viewing a plain text version of this content. The canonical link for it is here.
Posted to cvs@httpd.apache.org by co...@apache.org on 2007/08/17 19:33:13 UTC

svn commit: r567091 - in /httpd/httpd/trunk: CHANGES modules/ldap/util_ldap.c

Author: covener
Date: Fri Aug 17 10:33:11 2007
New Revision: 567091

URL: http://svn.apache.org/viewvc?view=rev&rev=567091
Log:
AFAICT, LDAP_CACHE_LOCK was a no-op when virtualhosts were used 


Modified:
    httpd/httpd/trunk/CHANGES
    httpd/httpd/trunk/modules/ldap/util_ldap.c

Modified: httpd/httpd/trunk/CHANGES
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/CHANGES?view=diff&rev=567091&r1=567090&r2=567091
==============================================================================
--- httpd/httpd/trunk/CHANGES [utf-8] (original)
+++ httpd/httpd/trunk/CHANGES [utf-8] Fri Aug 17 10:33:11 2007
@@ -1,6 +1,9 @@
                                                         -*- coding: utf-8 -*-
 Changes with Apache 2.3.0
 
+  *) mod_ldap: Copy cache lock into per-server config
+     [Eric Covener]
+
   *) mod_negotiation: preserve Query String in resolving a type map
      PR 33112 [Jørgen Thomsen <apache jth.net>, Nick Kew]
 

Modified: httpd/httpd/trunk/modules/ldap/util_ldap.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/ldap/util_ldap.c?view=diff&rev=567091&r1=567090&r2=567091
==============================================================================
--- httpd/httpd/trunk/modules/ldap/util_ldap.c (original)
+++ httpd/httpd/trunk/modules/ldap/util_ldap.c Fri Aug 17 10:33:11 2007
@@ -2111,6 +2111,7 @@
     st->search_cache_size = base->search_cache_size;
     st->compare_cache_ttl = base->compare_cache_ttl;
     st->compare_cache_size = base->compare_cache_size;
+    st->util_ldap_cache_lock = base->util_ldap_cache_lock; 
 
     st->connections = NULL;
     st->ssl_supported = 0;

Re: svn commit: r567091 - in /httpd/httpd/trunk: CHANGES modules/ldap/util_ldap.c

Posted by Jim Jagielski <ji...@jaguNET.com>.
On Aug 17, 2007, at 2:12 PM, Jeff Trawick wrote:

> On 8/17/07, Jim Jagielski <ji...@jagunet.com> wrote:
>>
>> On Aug 17, 2007, at 1:46 PM, Eric Covener wrote:
>>
>>> On 8/17/07, Jim Jagielski <ji...@jagunet.com> wrote:
>>>> Does this change really require a CHANGES entry??
>>>>
>>>> covener@apache.org wrote:
>>>>>
>>>>> Author: covener
>>>>> Date: Fri Aug 17 10:33:11 2007
>>>>> New Revision: 567091
>>>>>
>>>>> URL: http://svn.apache.org/viewvc?view=rev&rev=567091
>>>>> Log:
>>>>> AFAICT, LDAP_CACHE_LOCK was a no-op when virtualhosts were used
>>>
>>> I can expand or remove it; there's crash/hang potential  
>>> (observed) as
>>> the cache code goes into shared memory.
>>>
>>
>> How about creating a PR about it and then closing it out.
>> That way it's a documented bug that is closed; if it
>> affects people, the PR will provide insight into what
>> was wrong and what was fixed, and the CHANGES entry
>> can be adjusted to reflect the PR number.
>
> I'm confused.  Visually it doesn't look like Lotus Notes where I'm
> reading this, but the message better fits &employer; corporate
> standards.  (And yes, in that other world I'd want a formal external
> description of the problem with nice text for customers and support to
> use to know exactly what conditions they need the fix and what is
> changed and so on).  That's very expensive, and also something that
> hasn't been done in the past when a developer has a fix to commit.
>

The issue is how does someone know if the fix in
CHANGES applies to them? As it is currently
written, they don't. There's no "background"
to the bug... In fact, it doesn't even "read"
*as* a bug. This needs to be changed. Either
a better bug description in CHANGES or else
put the bug description in Bugzilla and then
close it out.

Re: svn commit: r567091 - in /httpd/httpd/trunk: CHANGES modules/ldap/util_ldap.c

Posted by Jeff Trawick <tr...@gmail.com>.
On 8/17/07, Jim Jagielski <ji...@jagunet.com> wrote:
>
> On Aug 17, 2007, at 1:46 PM, Eric Covener wrote:
>
> > On 8/17/07, Jim Jagielski <ji...@jagunet.com> wrote:
> >> Does this change really require a CHANGES entry??
> >>
> >> covener@apache.org wrote:
> >>>
> >>> Author: covener
> >>> Date: Fri Aug 17 10:33:11 2007
> >>> New Revision: 567091
> >>>
> >>> URL: http://svn.apache.org/viewvc?view=rev&rev=567091
> >>> Log:
> >>> AFAICT, LDAP_CACHE_LOCK was a no-op when virtualhosts were used
> >
> > I can expand or remove it; there's crash/hang potential (observed) as
> > the cache code goes into shared memory.
> >
>
> How about creating a PR about it and then closing it out.
> That way it's a documented bug that is closed; if it
> affects people, the PR will provide insight into what
> was wrong and what was fixed, and the CHANGES entry
> can be adjusted to reflect the PR number.

I'm confused.  Visually it doesn't look like Lotus Notes where I'm
reading this, but the message better fits &employer; corporate
standards.  (And yes, in that other world I'd want a formal external
description of the problem with nice text for customers and support to
use to know exactly what conditions they need the fix and what is
changed and so on).  That's very expensive, and also something that
hasn't been done in the past when a developer has a fix to commit.

Re: svn commit: r567091 - in /httpd/httpd/trunk: CHANGES modules/ldap/util_ldap.c

Posted by Jim Jagielski <ji...@jaguNET.com>.
On Aug 17, 2007, at 1:46 PM, Eric Covener wrote:

> On 8/17/07, Jim Jagielski <ji...@jagunet.com> wrote:
>> Does this change really require a CHANGES entry??
>>
>> covener@apache.org wrote:
>>>
>>> Author: covener
>>> Date: Fri Aug 17 10:33:11 2007
>>> New Revision: 567091
>>>
>>> URL: http://svn.apache.org/viewvc?view=rev&rev=567091
>>> Log:
>>> AFAICT, LDAP_CACHE_LOCK was a no-op when virtualhosts were used
>
> I can expand or remove it; there's crash/hang potential (observed) as
> the cache code goes into shared memory.
>

How about creating a PR about it and then closing it out.
That way it's a documented bug that is closed; if it
affects people, the PR will provide insight into what
was wrong and what was fixed, and the CHANGES entry
can be adjusted to reflect the PR number.

Re: svn commit: r567091 - in /httpd/httpd/trunk: CHANGES modules/ldap/util_ldap.c

Posted by Jeff Trawick <tr...@gmail.com>.
On 8/17/07, Eric Covener <co...@gmail.com> wrote:
> On 8/17/07, Jim Jagielski <ji...@jagunet.com> wrote:
> > Does this change really require a CHANGES entry??
> >
> > covener@apache.org wrote:
> > >
> > > Author: covener
> > > Date: Fri Aug 17 10:33:11 2007
> > > New Revision: 567091
> > >
> > > URL: http://svn.apache.org/viewvc?view=rev&rev=567091
> > > Log:
> > > AFAICT, LDAP_CACHE_LOCK was a no-op when virtualhosts were used
>
> I can expand or remove it; there's crash/hang potential (observed) as
> the cache code goes into shared memory.

can't remove; problem was the wording

*) Fix crashes, high CPU loops, or potentially other problems in mod_ldap
  by properly handling the cache lock when vhost configs are merged.

or something like that

Re: svn commit: r567091 - in /httpd/httpd/trunk: CHANGES modules/ldap/util_ldap.c

Posted by Eric Covener <co...@gmail.com>.
On 8/17/07, Jim Jagielski <ji...@jagunet.com> wrote:
> Does this change really require a CHANGES entry??
>
> covener@apache.org wrote:
> >
> > Author: covener
> > Date: Fri Aug 17 10:33:11 2007
> > New Revision: 567091
> >
> > URL: http://svn.apache.org/viewvc?view=rev&rev=567091
> > Log:
> > AFAICT, LDAP_CACHE_LOCK was a no-op when virtualhosts were used

I can expand or remove it; there's crash/hang potential (observed) as
the cache code goes into shared memory.

-- 
Eric Covener
covener@gmail.com

Re: svn commit: r567091 - in /httpd/httpd/trunk: CHANGES modules/ldap/util_ldap.c

Posted by Jim Jagielski <ji...@jaguNET.com>.
Does this change really require a CHANGES entry??

covener@apache.org wrote:
> 
> Author: covener
> Date: Fri Aug 17 10:33:11 2007
> New Revision: 567091
> 
> URL: http://svn.apache.org/viewvc?view=rev&rev=567091
> Log:
> AFAICT, LDAP_CACHE_LOCK was a no-op when virtualhosts were used 
> 
> 
> Modified:
>     httpd/httpd/trunk/CHANGES
>     httpd/httpd/trunk/modules/ldap/util_ldap.c
> 
> Modified: httpd/httpd/trunk/CHANGES
> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/CHANGES?view=diff&rev=567091&r1=567090&r2=567091
> ==============================================================================
> --- httpd/httpd/trunk/CHANGES [utf-8] (original)
> +++ httpd/httpd/trunk/CHANGES [utf-8] Fri Aug 17 10:33:11 2007
> @@ -1,6 +1,9 @@
>                                                          -*- coding: utf-8 -*-
>  Changes with Apache 2.3.0
>  
> +  *) mod_ldap: Copy cache lock into per-server config
> +     [Eric Covener]
> +
>    *) mod_negotiation: preserve Query String in resolving a type map
>       PR 33112 [Jørgen Thomsen <apache jth.net>, Nick Kew]
>  
> 
> Modified: httpd/httpd/trunk/modules/ldap/util_ldap.c
> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/ldap/util_ldap.c?view=diff&rev=567091&r1=567090&r2=567091
> ==============================================================================
> --- httpd/httpd/trunk/modules/ldap/util_ldap.c (original)
> +++ httpd/httpd/trunk/modules/ldap/util_ldap.c Fri Aug 17 10:33:11 2007
> @@ -2111,6 +2111,7 @@
>      st->search_cache_size = base->search_cache_size;
>      st->compare_cache_ttl = base->compare_cache_ttl;
>      st->compare_cache_size = base->compare_cache_size;
> +    st->util_ldap_cache_lock = base->util_ldap_cache_lock; 
>  
>      st->connections = NULL;
>      st->ssl_supported = 0;
> 
> 


-- 
===========================================================================
   Jim Jagielski   [|]   jim@jaguNET.com   [|]   http://www.jaguNET.com/
	    "If you can dodge a wrench, you can dodge a ball."

Re: svn commit: r567091 - in /httpd/httpd/trunk: CHANGES modules/ldap/util_ldap.c

Posted by Jim Jagielski <ji...@jaguNET.com>.
Does this change really require a CHANGES entry??

covener@apache.org wrote:
> 
> Author: covener
> Date: Fri Aug 17 10:33:11 2007
> New Revision: 567091
> 
> URL: http://svn.apache.org/viewvc?view=rev&rev=567091
> Log:
> AFAICT, LDAP_CACHE_LOCK was a no-op when virtualhosts were used 
> 
> 
> Modified:
>     httpd/httpd/trunk/CHANGES
>     httpd/httpd/trunk/modules/ldap/util_ldap.c
> 
> Modified: httpd/httpd/trunk/CHANGES
> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/CHANGES?view=diff&rev=567091&r1=567090&r2=567091
> ==============================================================================
> --- httpd/httpd/trunk/CHANGES [utf-8] (original)
> +++ httpd/httpd/trunk/CHANGES [utf-8] Fri Aug 17 10:33:11 2007
> @@ -1,6 +1,9 @@
>                                                          -*- coding: utf-8 -*-
>  Changes with Apache 2.3.0
>  
> +  *) mod_ldap: Copy cache lock into per-server config
> +     [Eric Covener]
> +
>    *) mod_negotiation: preserve Query String in resolving a type map
>       PR 33112 [Jørgen Thomsen <apache jth.net>, Nick Kew]
>  
> 
> Modified: httpd/httpd/trunk/modules/ldap/util_ldap.c
> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/ldap/util_ldap.c?view=diff&rev=567091&r1=567090&r2=567091
> ==============================================================================
> --- httpd/httpd/trunk/modules/ldap/util_ldap.c (original)
> +++ httpd/httpd/trunk/modules/ldap/util_ldap.c Fri Aug 17 10:33:11 2007
> @@ -2111,6 +2111,7 @@
>      st->search_cache_size = base->search_cache_size;
>      st->compare_cache_ttl = base->compare_cache_ttl;
>      st->compare_cache_size = base->compare_cache_size;
> +    st->util_ldap_cache_lock = base->util_ldap_cache_lock; 
>  
>      st->connections = NULL;
>      st->ssl_supported = 0;
> 
> 


-- 
===========================================================================
   Jim Jagielski   [|]   jim@jaguNET.com   [|]   http://www.jaguNET.com/
	    "If you can dodge a wrench, you can dodge a ball."