httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Steffen <i...@apachelounge.com>
Subject Re: mpm_winnt lingering close
Date Tue, 11 Apr 2017 14:57:32 GMT

Running now with the Patch from Bill, no warnings seen sofar.


On Tuesday 11/04/2017 at 16:35, Stefan Eissing  wrote:
> well, your change just reverses the order of check and call. far be it 
> from me to say what is better.  just wanted to point that out.
>
>>
>> Am 11.04.2017 um 16:14 schrieb William A Rowe Jr 
>> <wrowe@rowe-clan.net>:
>>
>> Steffen, I think we are missing an edge case, if you could retest with
>> http://svn.apache.org/viewvc?view=revision&revision=1790978
>>
>> http://svn.apache.org/viewvc/httpd/httpd/trunk/server/mpm/winnt/child.c?r1=1758310&r2=1790978&view=patch
>> is the net patch between the current 2.4.25 and now.
>>
>> Stefan - if the socket is closed without disconnecting during 
>> lingering close,
>> I think we end up with stale socket handles in your original fix.
>>
>> On Tue, Apr 11, 2017 at 8:43 AM, Stefan Eissing
>> <stefan.eissing@greenbytes.de> wrote:
>>>
>>> Bill, submitted in trunk and proposed for 2.4.x. Thanks for testing.
>>>
>>> -Stefan
>>>
>>>>
>>>> Am 11.04.2017 um 15:30 schrieb William A Rowe Jr 
>>>> <wrowe@rowe-clan.net>:
>>>>
>>>> Great news, thanks Steffen!
>>>>
>>>> Stefan - if you apply to trunk and 2.4.x (I'm already +1 on 
>>>> inspection) I have
>>>> regression testing on Windows to do today and tomorrow and can give 
>>>> this
>>>> a little more exercise.
>>>>
>>>> Cheers,
>>>>
>>>> Bill
>>>>
>>>>
>>>>>
>>>>> On Tue, Apr 11, 2017 at 5:27 AM, Steffen <info@apachelounge.com>

>>>>> wrote:
>>>>> Running now with Patch  on AL windows with v1.10.1-git, nghttp2 1.21.1

>>>>> , no
>>>>> warning anymore seen.
>>>>>
>>>>>
>>>>>
>>>>> On Tuesday 11/04/2017 at 10:23, Stefan Eissing wrote:
>>>>>
>>>>>
>>>>> Am 03.04.2017 um 21:17 schrieb William A Rowe Jr 
>>>>> <wrowe@rowe-clan.net>:
>>>>>
>>>>> On Mon, Apr 3, 2017 at 8:21 AM, Eric Covener <covener@gmail.com>

>>>>> wrote:
>>>>>
>>>>> On Mon, Apr 3, 2017 at 9:07 AM, Stefan Eissing
>>>>> <stefan.eissing@greenbytes.de> wrote:
>>>>>
>>>>> Question is: do we "fix" mpm_winnt or is there a better way for 
>>>>> mod_http2 to
>>>>> shutdown the connection before mod_ssl does. This would need to work

>>>>> in
>>>>> async mpms for any connection state.
>>>>>
>>>>>
>>>>> I think it's okay to add the prep cal when short-circuiting lingering
>>>>> close, but it seems like very little extra will be running in the full
>>>>> call either.
>>>>>
>>>>>
>>>>> Jeff Trawick knew this bit of logic better than most any of us, I'd 
>>>>> love
>>>>> to hear his thoughts on the cleanest solution. But fixing mpm_winnt
>>>>> to behave as the other MPM's would be worthwhile. It's also worth
>>>>> looking at third party MPM's such as mpm_itk to see if we have an
>>>>> underlying bug that must be fixed.
>>>>>
>>>>> I presume we still allow the disconnected socket to be recycled, which
>>>>> was the underlying idea behind the shortcut/optimization. It seems 
>>>>> that
>>>>> the shortcut isn't (valid).
>>>>>
>>>>>
>>>>> All other of our mpms in trunk call ap_lingering_close() 
>>>>> unconditionally
>>>>> after ap_process_connection(). So, mod_http2 should be fine there.
>>>>>
>>>>> mpm_itk, in the latest release at least, also always invokes
>>>>> ap_lingering_close() before terminating a connection.
>>>>>
>>>>>
>>>>> I propose the following patch for trunk and, if it does indeed work
>>>>> as expected will propose backport to 2.4.x. Since I do not run a
>>>>> Windows build, I'd like to hear peoples opinion and need a tester.
>>>>>
>>>>> Index: server/mpm/winnt/child.c
>>>>> ===================================================================
>>>>> --- server/mpm/winnt/child.c (revision 1790842)
>>>>> +++ server/mpm/winnt/child.c (working copy)
>>>>> @@ -817,10 +817,8 @@
>>>>>
>>>>>                  if (!disconnected) {
>>>>>                          context->accept_socket = INVALID_SOCKET;
>>>>> - if (!c->aborted) {
>>>>> - ap_lingering_close(c);
>>>>> - }
>>>>>                  }
>>>>> + ap_lingering_close(c);
>>>>>          }
>>>>>
>>>>>          ap_update_child_status_from_indexes(0, thread_num, 
>>>>> SERVER_DEAD, NULL);
>>>>>
>>>>> Steffen, if you want to try this in your Windows build, I attached the
>>>>> file from 2.4.x with the patch applied.
>>>>>
>>>>>
>>>>>
>>>
>


Mime
  • Unnamed multipart/alternative (inline, Quoted Printable, 0 bytes)
View raw message