Return-Path: X-Original-To: apmail-httpd-dev-archive@www.apache.org Delivered-To: apmail-httpd-dev-archive@www.apache.org Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by minotaur.apache.org (Postfix) with SMTP id EC75ED9E5 for ; Thu, 16 May 2013 08:19:45 +0000 (UTC) Received: (qmail 93281 invoked by uid 500); 16 May 2013 08:19:45 -0000 Delivered-To: apmail-httpd-dev-archive@httpd.apache.org Received: (qmail 93009 invoked by uid 500); 16 May 2013 08:19:41 -0000 Mailing-List: contact dev-help@httpd.apache.org; run by ezmlm Precedence: bulk Reply-To: dev@httpd.apache.org list-help: list-unsubscribe: List-Post: List-Id: Delivered-To: mailing list dev@httpd.apache.org Received: (qmail 92951 invoked by uid 99); 16 May 2013 08:19:40 -0000 Received: from nike.apache.org (HELO nike.apache.org) (192.87.106.230) by apache.org (qpsmtpd/0.29) with ESMTP; Thu, 16 May 2013 08:19:40 +0000 X-ASF-Spam-Status: No, hits=1.5 required=5.0 tests=HTML_MESSAGE,RCVD_IN_DNSWL_LOW,SPF_PASS X-Spam-Check-By: apache.org Received-SPF: pass (nike.apache.org: domain of thomas.r.w.eckert@gmail.com designates 209.85.217.170 as permitted sender) Received: from [209.85.217.170] (HELO mail-lb0-f170.google.com) (209.85.217.170) by apache.org (qpsmtpd/0.29) with ESMTP; Thu, 16 May 2013 08:19:34 +0000 Received: by mail-lb0-f170.google.com with SMTP id t13so80086lbd.1 for ; Thu, 16 May 2013 01:19:14 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:x-received:in-reply-to:references:date:message-id :subject:from:to:content-type; bh=jbHQ3twfWJE8narpM/v3WcDdArsGKZ/ma5g7G2GT6Gw=; b=EZwOOkKaXdQm0nEyQG/VWxKz/pgo65FMZZdQwnDDC5xlD7v5XY521G2f/PptTAdV4K A3qerDA9iinj/icdlSzI2PAquGxqrRBxzXFxyMWj6JV/TWD3Qwho6ccNd2Ge9cJpcJ7X dQ+XLZRWUDoKNiN1rIh5kiVYKTkJsDseMtmQBxzgZ7gBKykegv6bXQUIhcoaaX9mgoBI HCONB34W7uFsQYDAnEy6lLORb0fSKrKZUE2dwMBRs7jqpg57YGKx5Icg0kj2XHnwC8N5 BciMk+a/wBujMku6NIYZS+ZYiHctBBLyVWFu0aa3TwETBR57CHCPQHY7FXa9+ELPN0Nj +r4g== MIME-Version: 1.0 X-Received: by 10.112.218.102 with SMTP id pf6mr7419080lbc.77.1368692354408; Thu, 16 May 2013 01:19:14 -0700 (PDT) Received: by 10.112.125.69 with HTTP; Thu, 16 May 2013 01:19:14 -0700 (PDT) In-Reply-To: <46F545B7-56DB-41E5-AF44-5BE6E75AC956@jaguNET.com> References: <78FB8419-356C-407B-93A8-830D8DD88EAB@sharp.fm> <0CB43A52-AFE3-426B-B117-4B9CE8DDBE62@webthing.com> <78D0C435-6E2C-4990-9200-1777CD911EFF@jaguNET.com> <347A9ACF-AF52-4253-968A-223B615256B2@sharp.fm> <98E85B1D-1B18-43A0-BF48-ACFEFFDBB971@jaguNET.com> <46F545B7-56DB-41E5-AF44-5BE6E75AC956@jaguNET.com> Date: Thu, 16 May 2013 10:19:14 +0200 Message-ID: Subject: Re: proxy segfault in httpd-trunk From: Thomas Eckert To: dev@httpd.apache.org Content-Type: multipart/alternative; boundary=001a11c3dd849764ef04dcd18563 X-Virus-Checked: Checked by ClamAV on apache.org --001a11c3dd849764ef04dcd18563 Content-Type: text/plain; charset=ISO-8859-1 > Just wondering if we also have a problem with the pool > as well... if base doesn't have a proxy, we don't have > the subpool. Looks like it. At least I don't see a reason why Nick's reasoning would apply to the mutex but not to the pool. > BTW, wondering if instead of leaking proxy_mutex we > could set ps->mutex = proxy_mutex in mod_proxy.c when > we merge. We could then make proxy_mutex static...? I must admit I'm not familiar enough with the httpd modules at large to be an expert here but to me it *feels* weird to put such a central piece of module management outside the module's config structure. It would solve the problem with create_config/merge_config though and it's also a bit better performance wise (see Graham's commit). Hm, maybe it doesn't feel so weird after all ... > Hmmm... The other idea is to keep it as it was, > stick pconf back in conf->pool but just always create > a sub-pool before conf->pool is used. This *looks* > like it removes the need for a mutex... I thought about this as well but decided against it because I figured creating those sub pools would be a much larger performance hit then just having a locking mechanism in place. It'd be a different matter with sub pools being pre-allocated, 'swapped' into place when needed, etc. This feels like going way overboard though. On Thu, May 16, 2013 at 5:20 AM, Jim Jagielski wrote: > Hmmm... The other idea is to keep it as it was, > stick pconf back in conf->pool but just always create > a sub-pool before conf->pool is used. This *looks* > like it removes the need for a mutex... > > On May 15, 2013, at 7:37 PM, Jim Jagielski wrote: > > > Just wondering if we also have a problem with the pool > > as well... if base doesn't have a proxy, we don't have > > the subpool. > > > > BTW, wondering if instead of leaking proxy_mutex we > > could set ps->mutex = proxy_mutex in mod_proxy.c when > > we merge. We could then make proxy_mutex static...? > > > > On May 15, 2013, at 7:27 PM, Graham Leggett wrote: > > > >> On 16 May 2013, at 1:25 AM, Jim Jagielski wrote: > >> > >>> Ugg. You're 100% right. We need to create a global. > >> > >> Here is one I made earlier: http://svn.apache.org/r1482859 > >> > >> Regards, > >> Graham > >> -- > >> > > > > --001a11c3dd849764ef04dcd18563 Content-Type: text/html; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable
> Just wondering if we also have a problem wi= th the pool
> as well... if base doesn't have a proxy, we don'= ;t have
> the subpool.

Looks like it. At leas= t I don't see a reason why Nick's reasoning would apply to the mute= x but not to the pool.


> BTW, wondering if instead of leaking proxy_mutex we
> co= uld set ps->mutex =3D proxy_mutex in mod_proxy.c when
> we merge. = We could then make proxy_mutex static...?
I must admit I'm not familiar enough with the httpd modules at large= to be an expert here but to me it *feels* weird to put such a central piec= e of module management outside the module's config structure. It would = solve the problem with create_config/merge_config though and it's also = a bit better performance wise (see Graham's commit). Hm, maybe it doesn= 't feel so weird after all ...


> Hmmm... The other idea is to keep it as it was,
> stick = pconf back in conf->pool but just always create
> a sub-pool befor= e conf->pool is used. This *looks*
> like it removes the need for = a mutex...

I thought about this as well but decided against = it because I figured creating those sub pools would be a much larger perfor= mance hit then just having a locking mechanism in place. It'd be a diff= erent matter with sub pools being pre-allocated, 'swapped' into pla= ce when needed, etc. This feels like going way overboard though.


O= n Thu, May 16, 2013 at 5:20 AM, Jim Jagielski <jim@jagunet.com> wrote:
Hmmm... The other idea is to keep it as it w= as,
stick pconf back in conf->pool but just always create
a sub-pool before conf->pool is used. This *looks*
like it removes the need for a mutex...

On May 15, 2013, at 7:37 PM, Jim Jagielski <jim@jaguNET.com> wrote:
> Just wondering if we also have a problem with the pool
> as well... if base doesn't have a proxy, we don't have
> the subpool.
>
> BTW, wondering if instead of leaking proxy_mutex we
> could set ps->mutex =3D proxy_mutex in mod_proxy.c when
> we merge. We could then make proxy_mutex static...?
>
> On May 15, 2013, at 7:27 PM, Graham Leggett <minfrin@sharp.fm> wrote:
>
>> On 16 May 2013, at 1:25 AM, Jim Jagielski <jim@jaguNET.com> = wrote:
>>
>>> Ugg. You're 100% right. We need to create a global.
>>
>> Here is one I made earlier: http://svn.apache.org/r1482859
>>
>> Regards,
>> Graham
>> --
>>
>


--001a11c3dd849764ef04dcd18563--