You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Justin Erenkrantz <je...@ebuilt.com> on 2001/08/24 07:28:37 UTC

Re: cvs commit: httpd-2.0/modules/ssl README mod_ssl.h ssl_engine_init.c ssl_util.c

On Fri, Aug 24, 2001 at 04:08:04AM -0000, dougm@apache.org wrote:
>   +    *lock_cs = apr_palloc(p, CRYPTO_NUM_LOCKS);
>   +    for (i = 0; i < CRYPTO_NUM_LOCKS; i++)
>   +    {
>   +        lock_count[i]=0;
>   +        apr_lock_create(&(lock_cs[i]), APR_MUTEX, APR_LOCKALL,
>   +                                                mc->szMutexFile, p);
>   +    }

I meant to review this patch, but I accidentally hit the delete key on 
the original message.  Anyway, that APR_LOCKALL can be APR_INTRAPROCESS.
We're only concerned about safety within our process not across 
multiple processes - there is no need to needlessly lock across
processes here.

IMHO, the call to ssl_util_thread_setup should be moved to 
ssl_init_Child and even potentially surrounded by 
#if APR_HAS_THREADS.  -- justin


Re: cvs commit: httpd-2.0/modules/ssl README mod_ssl.h ssl_engine_init.c ssl_util.c

Posted by Doug MacEachern <do...@covalent.net>.
On Thu, 23 Aug 2001, Justin Erenkrantz wrote:
 
> If you created the locks as INTRAPROCESS before fork()ing new children, 
> would each children's copy of the lock be isolated?  Maybe.  If so, 
> then the post_config hook works, but I think child_init makes a bit 
> more sense.  But, that's me.  -- justin

there must be other intraprocess locks created at startup time, i'd be
surprised if there's an issue.  one problem with using child init is that
not all mpms support it, notice theres a few missing:
% find . -name "*.c" | xargs grep ap_run_child_init 
./server/mpm/perchild/perchild.c:    ap_run_child_init(pchild, ap_server_conf);
./server/mpm/prefork/prefork.c:    ap_run_child_init(pchild, ap_server_conf);
./server/mpm/spmt_os2/spmt_os2.c:    ap_run_child_init(pchild, ap_server_conf);
./server/mpm/threaded/threaded.c:    ap_run_child_init(pchild, ap_server_conf);
./server/mpm/worker/worker.c:    ap_run_child_init(pchild, ap_server_conf);


Re: cvs commit: httpd-2.0/modules/ssl README mod_ssl.h ssl_engine_init.c ssl_util.c

Posted by Justin Erenkrantz <je...@ebuilt.com>.
On Thu, Aug 23, 2001 at 10:42:16PM -0700, Doug MacEachern wrote:
> > IMHO, the call to ssl_util_thread_setup should be moved to 
> > ssl_init_Child and even potentially surrounded by 
> > #if APR_HAS_THREADS.  -- justin
> 
> why child init rather than module init?
> i did turn off the locking unless the mpm is threaded.  isn't it possible
> that APR_HAS_THREADS is true even though the mpm is not threaded?

Yes, you are correct here.  I didn't think about the mpm query
facilities.  =)

It should be in child init rather than module init so that the locks are
created per child not globally (via post_config hook).  That is the 
tradeoff by having APR_INTRAPROCESS.  

If you created the locks as INTRAPROCESS before fork()ing new children, 
would each children's copy of the lock be isolated?  Maybe.  If so, 
then the post_config hook works, but I think child_init makes a bit 
more sense.  But, that's me.  -- justin


Re: cvs commit: httpd-2.0/modules/ssl README mod_ssl.h ssl_engine_init.c ssl_util.c

Posted by Doug MacEachern <do...@covalent.net>.
On Thu, 23 Aug 2001, Justin Erenkrantz wrote:
 
> I meant to review this patch, but I accidentally hit the delete key on 
> the original message.  Anyway, that APR_LOCKALL can be APR_INTRAPROCESS.
> We're only concerned about safety within our process not across 
> multiple processes - there is no need to needlessly lock across
> processes here.

right, thanks, that change is in.
 
> IMHO, the call to ssl_util_thread_setup should be moved to 
> ssl_init_Child and even potentially surrounded by 
> #if APR_HAS_THREADS.  -- justin

why child init rather than module init?
i did turn off the locking unless the mpm is threaded.  isn't it possible
that APR_HAS_THREADS is true even though the mpm is not threaded?