Balancers (file based SHMs) and restart issue (Windows only?)

[Yann Ylavic]

Re PR 58024.

AIUI, balancers (and members) SHM slots are destroyed (and the
underlying base file removed) with pconf before restarting and then
re-created after (according to the new configuration, eg. number of
balancers/members, be it the same or not).
Persisted slots are saved in their own file (with the .persit suffix),
and reused (copied) only if the configuration did not change in
between.
(@Jim, I see now how this is better than attaching ;)

This works well on Unixes, but on Windows files can't be removed
(unlinked) while any process/thread hold an HANDLE on it, hence until
all the children have exited...
Thus, since the SHM files still exist on restart, the balancer code
tries to attach them instead of re-creating, and issues the checks on
the existing size to fit the reloaded configuration, and fail should
should any balancer/member be added or removed => PR 58024.

BTW, even on Unixes there is a race on these files between the main
process and the children, or the children themselves from different
"generation".

So I wonder if we could use anonymous SHMs instead (or mktemp based
when not native), persisted slots are saved in their own files anyway
and won't be affected.
Otherwise, maybe we could use the "generation" as part of the file
name, but that's equivalent and more complex IMO.

Thoughts?

Re: svn commit: r1695727 - in /httpd/httpd/trunk: docs/manual/mod/core.xml include/http_core.h server/core.c server/protocol.c

[Yann Ylavic]

On Thu, Aug 13, 2015 at 5:33 PM,   wrote:
> Author: icing
> Date: Thu Aug 13 15:33:07 2015
> New Revision: 1695727
>
> URL: http://svn.apache.org/r1695727
> Log:
> new directive ProtocolsHonorOrder, added documentation for Protocols feature, 
> changed preference selection and config merging
>
> Modified:
> httpd/httpd/trunk/docs/manual/mod/core.xml
[]
>
> Modified: httpd/httpd/trunk/docs/manual/mod/core.xml
> URL: 
> http://svn.apache.org/viewvc/httpd/httpd/trunk/docs/manual/mod/core.xml?rev=1695727=1695726=1695727=diff
> ==
> --- httpd/httpd/trunk/docs/manual/mod/core.xml (original)
> +++ httpd/httpd/trunk/docs/manual/mod/core.xml Thu Aug 13 15:33:07 2015
> @@ -3711,6 +3711,71 @@ Protocol https
>
>
>  
> +Protocols
> +Protocols available for a server/virtual host
> +Protocols protocol ...
> +server configvirtual 
> host
> +Only available from Apache 2.4.17 and 
> later.
> +
> +
> +This directive specifies the list of protocols supported for a
> +server/virtual host. The list determines the allowed protocols
> +a client may negotiate for this server/host.
> +
> +You only need to set protocols if you want to limit the available
> +protocols for a server/host. By default, all supported protocols
> +are available to a client.
> +
> +For example, if you want to support only HTTP/1.1 for a server, 
> even
> +though HTTP/2 is available, just specify this protocol only:
> +
> +
> +Protocols http/1.1
> +

It is not clear to me (still, see [1]) why Protocols would default
like this, and h2 be available for all the vhosts (provided mod_http2
is loaded) unless the above is configured.
IOW, I'd prefer "Protocols http/1.1" to be the default (at least for 2.4.x).

Also, since "http/1.1" is implicit, and ssl_callback_alpn_select() is
unconditionally registered, we can end up negociating proposing
"http/1.1" with the client even if is was not asked (and using ALPN
extensions when not needed).
Couldn't we instead either not register the callback or return
SSL_TLSEXT_ERR_NOACK when no protocol is selected (including when no
Protocols is configured).

I think POLS suggests that...

[]
> +
> +
> +
> +
> +ProtocolsHonorOrder
> +Protocols available for a server/virtual host
> +ProtocolsHonorOrder On|Off
> +ProtocolsHonorOrder Off

Again here I'd suggest "On" by default (eg. SSLHonorCipherOrder is
quite recommended today, if that's a valid analogy).

[]
> +

Regards,
Yann.

[1] http://www.mail-archive.com/dev%40httpd.apache.org/msg62160.html

Re: svn commit: r1695727 - in /httpd/httpd/trunk: docs/manual/mod/core.xml include/http_core.h server/core.c server/protocol.c

[Stefan Eissing]

We can choose any string we want, but for ALPN negotiation there is only 
http/1.1 defined. We could of course translate that but then we need to explain 
somewhere what we mean by it.

For the Upgrade case, we could ap_get_protocol make return exactly what the 
client sent us. But accomodating for http/1.0 and 0.9 in case of Upgrade sounds 
rather esoteric. I would rather keep it simple. The exact http version is 
available at the request_rec for anyone who really cares. 

> Am 02.09.2015 um 18:53 schrieb William A Rowe Jr :
> 
>> On Wed, Sep 2, 2015 at 11:20 AM, Yann Ylavic  wrote:
>> 
>> It is not clear to me (still, see [1]) why Protocols would default
>> like this, and h2 be available for all the vhosts (provided mod_http2
>> is loaded) unless the above is configured.
>> IOW, I'd prefer "Protocols http/1.1" to be the default (at least for 2.4.x).
>> 
>> Also, since "http/1.1" is implicit, and ssl_callback_alpn_select() is
>> unconditionally registered, we can end up negociating proposing
>> "http/1.1" with the client even if is was not asked (and using ALPN
>> extensions when not needed).
> 
> Just a side thought... we are still honoring http/1.0.  Wouldn't http/1 be 
> more reflective of the state of the protocol (e.g. http/1.0 or http/1.1 are 
> honored)?
> 
> Or do we intend to distinguish these and permit only one or the other 
> (in which case, http/1 might still be correct in this case, but the user 
> could alternately specify http/1.1 to permit only 1.1 negotiation?)
> 

Re: svn commit: r1692486 [1/2] - in /httpd/httpd/trunk: docs/log-message-tags/ include/ modules/http2/ modules/ssl/ server/

[Yann Ylavic]

On Wed, Sep 2, 2015 at 4:31 PM, Stefan Eissing
 wrote:
>
>> Am 02.09.2015 um 16:19 schrieb Yann Ylavic :
>>
>> On Wed, Sep 2, 2015 at 4:04 PM, Eric Covener  wrote:
>>> On Wed, Sep 2, 2015 at 10:01 AM, Stefan Eissing
>>>  wrote:
 Hmm. Is the
  ap_hook_handler(core_upgrade_handler,NULL,NULL,APR_HOOK_REALLY_FIRST);
 called for requests that mod_proxy_wstunnel makes against a backend?
>>>
>>> yes, the guts of mod_proxy is just a handler, so this would run before that.
>>
>> Maybe core_upgrade_handler could check if the Upgrade's protocol is in
>> Protocols, and do nothing otherwise...
>
> It calls ap_select_protocol(...) and that one allows only selections from the 
> configured "Protocols" list.

Hm ok, so here (with "Upgrade: WebSocket" but no "Protocols
WebSocket"), ap_select_protocol() would return "http/1.1" which is the
current protocol, hence we would not enter the
HTTP_SWITCHING_PROTOCOLS code.

I had misread the strcmp() and thought the selected protocol was
compared against the proposed one (Upgrade header).

Thanks for the clarification.

Re: svn commit: r1695727 - in /httpd/httpd/trunk: docs/manual/mod/core.xml include/http_core.h server/core.c server/protocol.c

[Eric Covener]

On Wed, Sep 2, 2015 at 12:20 PM, Yann Ylavic  wrote:
> Again here I'd suggest "On" by default (eg. SSLHonorCipherOrder is
> quite recommended today, if that's a valid analogy).


+1

-- 
Eric Covener
cove...@gmail.com

Re: svn commit: r1695727 - in /httpd/httpd/trunk: docs/manual/mod/core.xml include/http_core.h server/core.c server/protocol.c

[William A Rowe Jr]

On Wed, Sep 2, 2015 at 11:20 AM, Yann Ylavic  wrote:

>
> It is not clear to me (still, see [1]) why Protocols would default
> like this, and h2 be available for all the vhosts (provided mod_http2
> is loaded) unless the above is configured.
> IOW, I'd prefer "Protocols http/1.1" to be the default (at least for
> 2.4.x).
>
> Also, since "http/1.1" is implicit, and ssl_callback_alpn_select() is
> unconditionally registered, we can end up negociating proposing
> "http/1.1" with the client even if is was not asked (and using ALPN
> extensions when not needed).
>

Just a side thought... we are still honoring http/1.0.  Wouldn't http/1 be
more reflective of the state of the protocol (e.g. http/1.0 or http/1.1 are
honored)?

Or do we intend to distinguish these and permit only one or the other
(in which case, http/1 might still be correct in this case, but the user
could alternately specify http/1.1 to permit only 1.1 negotiation?)

Re: svn commit: r1695727 - in /httpd/httpd/trunk: docs/manual/mod/core.xml include/http_core.h server/core.c server/protocol.c

[Stefan Eissing]

If we want to be more safe, we can change the Protocols default to just 
http/1.1. Also the default for ordering we can change, np. 

Other opinions? 

For ALPN, afaik the callback only gets triggered if the client actually sends 
ALPN in its hello. Since "http/1.1" is the only identifier defined in the 
standard (for http version < 2), we cannot send any 1.0 or 0.9. And if the 
client does, it's an unidentified thing. ALPN says that the server is free to 
select even a protocol not mentioned in the client hello. So sending back 
"http/1.1" in case server/client wishes do not overlap is fine too. Either the 
client reconsiders or closes the connection. 

Legacy clients will not send ALPN, so the whole handshake will work as before. 
(modulo bugs)

> Am 02.09.2015 um 18:20 schrieb Yann Ylavic :
> 
>> On Thu, Aug 13, 2015 at 5:33 PM,   wrote:
>> Author: icing
>> Date: Thu Aug 13 15:33:07 2015
>> New Revision: 1695727
>> 
>> URL: http://svn.apache.org/r1695727
>> Log:
>> new directive ProtocolsHonorOrder, added documentation for Protocols 
>> feature, changed preference selection and config merging
>> 
>> Modified:
>>httpd/httpd/trunk/docs/manual/mod/core.xml
> []
>> 
>> Modified: httpd/httpd/trunk/docs/manual/mod/core.xml
>> URL: 
>> http://svn.apache.org/viewvc/httpd/httpd/trunk/docs/manual/mod/core.xml?rev=1695727=1695726=1695727=diff
>> ==
>> --- httpd/httpd/trunk/docs/manual/mod/core.xml (original)
>> +++ httpd/httpd/trunk/docs/manual/mod/core.xml Thu Aug 13 15:33:07 2015
>> @@ -3711,6 +3711,71 @@ Protocol https
>> 
>> 
>> 
>> +Protocols
>> +Protocols available for a server/virtual host
>> +Protocols protocol ...
>> +server configvirtual 
>> host
>> +Only available from Apache 2.4.17 and 
>> later.
>> +
>> +
>> +This directive specifies the list of protocols supported for a
>> +server/virtual host. The list determines the allowed protocols
>> +a client may negotiate for this server/host.
>> +
>> +You only need to set protocols if you want to limit the available
>> +protocols for a server/host. By default, all supported protocols
>> +are available to a client.
>> +
>> +For example, if you want to support only HTTP/1.1 for a server, 
>> even
>> +though HTTP/2 is available, just specify this protocol only:
>> +
>> +
>> +Protocols http/1.1
>> +
> 
> It is not clear to me (still, see [1]) why Protocols would default
> like this, and h2 be available for all the vhosts (provided mod_http2
> is loaded) unless the above is configured.
> IOW, I'd prefer "Protocols http/1.1" to be the default (at least for 2.4.x).
> 
> Also, since "http/1.1" is implicit, and ssl_callback_alpn_select() is
> unconditionally registered, we can end up negociating proposing
> "http/1.1" with the client even if is was not asked (and using ALPN
> extensions when not needed).
> Couldn't we instead either not register the callback or return
> SSL_TLSEXT_ERR_NOACK when no protocol is selected (including when no
> Protocols is configured).
> 
> I think POLS suggests that...
> 
> []
>> +
>> +
>> +
>> +
>> +ProtocolsHonorOrder
>> +Protocols available for a server/virtual host
>> +ProtocolsHonorOrder On|Off
>> +ProtocolsHonorOrder Off
> 
> Again here I'd suggest "On" by default (eg. SSLHonorCipherOrder is
> quite recommended today, if that's a valid analogy).
> 
> []
>> +
> 
> Regards,
> Yann.
> 
> [1] http://www.mail-archive.com/dev%40httpd.apache.org/msg62160.html

r1700777 - h2_stream.c

[NormW]

hi,
If not mistaken, on the latest httpd-trunk\modules\http2 update:


D:\Projects\svn\httpd-trunk\modules\http2>svn diff
Index: h2_session.c
===
--- h2_session.c(revision 1700915)
+++ h2_session.c(working copy)
@@ -259,8 +259,9 @@
const nghttp2_frame *frame, void *userp)
 {
 /* This starts a new stream. */
+int rv;
 (void)ngh2;
-int rv = stream_open((h2_session *)userp, frame->hd.stream_id);
+rv = stream_open((h2_session *)userp, frame->hd.stream_id);
 if (rv != NGHTTP2_ERR_CALLBACK_FAILURE) {
   /* on_header_cb or on_frame_recv_cb will dectect that stream
  does not exist and submit RST_STREAM. */


Otherwise compiles fine.
Norm

Re: r1700777 - h2_stream.c

[Gregg Smith]

Hi Norm,

I need it too.
done in r1700917

Gregg

On 9/2/2015 5:07 PM, NormW wrote:

hi,
If not mistaken, on the latest httpd-trunk\modules\http2 update:


D:\Projects\svn\httpd-trunk\modules\http2>svn diff
Index: h2_session.c
===
--- h2_session.c(revision 1700915)
+++ h2_session.c(working copy)
@@ -259,8 +259,9 @@
const nghttp2_frame *frame, void *userp)
 {
 /* This starts a new stream. */
+int rv;
 (void)ngh2;
-int rv = stream_open((h2_session *)userp, frame->hd.stream_id);
+rv = stream_open((h2_session *)userp, frame->hd.stream_id);
 if (rv != NGHTTP2_ERR_CALLBACK_FAILURE) {
   /* on_header_cb or on_frame_recv_cb will dectect that stream
  does not exist and submit RST_STREAM. */


Otherwise compiles fine.
Norm

Re: proposed backport of mod_h2

[Gregg Smith]

Hi Stefan,

core patch:
needs r1694950 and may require a minor mmn bump, at least that is what I 
took away from the discussion about it over this commit.


h2 patch:
needs r1700917

Thanks,

Gregg


On 9/2/2015 7:10 AM, Stefan Eissing wrote:

As in r1700829. Regression tests run (after small change on apache version 
check in h2.t) on OS X for me.

Cheers,

   Stefan

bytes GmbH
Hafenweg 16, 48155 Münster, Germany
Phone: +49 251 2807760. Amtsgericht Münster: HRB5782

Re: svn commit: r1698133 - in /httpd/httpd/trunk: include/httpd.h modules/http2/h2_switch.c server/protocol.c server/util.c

[Yann Ylavic]

On Thu, Aug 27, 2015 at 2:13 PM,   wrote:
>
> Modified: httpd/httpd/trunk/server/util.c
> URL: 
> http://svn.apache.org/viewvc/httpd/httpd/trunk/server/util.c?rev=1698133=1698132=1698133=diff
> ==
> --- httpd/httpd/trunk/server/util.c (original)
> +++ httpd/httpd/trunk/server/util.c Thu Aug 27 12:13:59 2015
> @@ -3149,15 +3149,23 @@ AP_DECLARE(char *) ap_get_exec_line(apr_
>  return apr_pstrndup(p, buf, k);
>  }
>
> -AP_DECLARE(int) ap_array_index(const apr_array_header_t *array, const char 
> *s)
> +AP_DECLARE(int) ap_array_index(const apr_array_header_t *array,
> +   const char *s,
> +   apr_size_t start)
>  {
> -int i;
> -for (i = 0; i < array->nelts; i++) {
> +apr_size_t i;

Maybe here:
if (start < 0) {
return -1;
}
?

> +for (i = start; i < array->nelts; i++) {
>  const char *p = APR_ARRAY_IDX(array, i, const char *);
>  if (!strcmp(p, s)) {
> -return i;
> +return (int)i;
>  }
>  }
>  return -1;
>  }

Re: svn commit: r1698133 - in /httpd/httpd/trunk: include/httpd.h modules/http2/h2_switch.c server/protocol.c server/util.c

[Yann Ylavic]

On Thu, Aug 27, 2015 at 5:06 PM, William A Rowe Jr  wrote:
>
> On Aug 27, 2015 7:14 AM,  wrote:
>>
>> Author: icing
>> Date: Thu Aug 27 12:13:59 2015
>> New Revision: 1698133
>>
>> URL: http://svn.apache.org/r1698133
>> Log:
>> giving ap_array_index a start parameter, adding ap_array_contains
>>
>
>>   */
>> -AP_DECLARE(int) ap_array_index(const apr_array_header_t *array, const
>> char *s);
>> +AP_DECLARE(int) ap_array_index(const apr_array_header_t *array,
>> +   const char *s,
>> +   apr_size_t start);
>
> You want the type of rv of _index to correspond to the start input to rv,
> no?  E.g.
>
> int n = -1, count = 0;
> while ((n = ap_array_index(arr, findtag, n + 1)) >= 0)
> ++count;
>
> sizeof(int) does not have to equal sizeof(apr_size_t).  But for indexes I
> believe it's fine.  The alternative is apr_ssize_t for both start arg and
> rv.

Or use the same type as array->nelts: int?
The conversion from unsigned to signed and (possible) sizeof mixing
looks incorrect to me.

Also, maybe ap_array_string_index would be a better name since arrays
contain any pointer (likewise
s/ap_array_contains/ap_array_has_string/g).
We could then create new ones for other types when/if necessary (or
add them to APR by appending a single r :)

Re: svn commit: r1692486 [1/2] - in /httpd/httpd/trunk: docs/log-message-tags/ include/ modules/http2/ modules/ssl/ server/

[Stefan Eissing]

But would it have an "Upgrade: + Connection:" header? I might be wrong, but I 
thought mod_proxy_wstunnel adds those headers before sending the request to a 
backend. So the core_ugprade_handler would not see those.

> Am 02.09.2015 um 16:04 schrieb Eric Covener :
> 
> On Wed, Sep 2, 2015 at 10:01 AM, Stefan Eissing
>  wrote:
>> Hmm. Is the
>>  ap_hook_handler(core_upgrade_handler,NULL,NULL,APR_HOOK_REALLY_FIRST);
>> called for requests that mod_proxy_wstunnel makes against a backend?
> 
> yes, the guts of mod_proxy is just a handler, so this would run before that.

bytes GmbH
Hafenweg 16, 48155 Münster, Germany
Phone: +49 251 2807760. Amtsgericht Münster: HRB5782

Re: [PATCH 57300] mod_session save optimization

[Paul Spangler]

On 8/20/2015 4:58 PM, Paul Spangler wrote:

Hello,

The bug report contains a more detailed explanation of the patch, but
there are some points I thought might lead to some discussion.

First a quick summary of the issue: mod_session writes out the session
every request even if there aren't any changes to the data. This makes
some sense when the session has a max age and the expiry value needs to
be refreshed. However, there isn't likely to be much benefit in
repeatedly refreshing the expiry by a few milliseconds, possibly
generating database traffic each time. This patch proposes a new
directive to define an interval of time where the expiry doesn't need to
be refreshed if there are no session data changes.

1. We had a hard time coming up with a name for the directive. The patch
goes with SessionExpiryUpdateInterval, being the interval of time that
may pass before updating the expiry value is required. I don't know if
there are any existing directives with a similar function that we should
mimic instead.

2. The patch includes a behavior change independent of the new directive
when using sessions without a max age: if the data hasn't changed, don't
write out the session. Most noticeably, this means new sessions that
never get data are discarded without being saved.

3. I wasn't sure how best to add tests for a new directive since the
test server won't start if the directive is missing. The patch that
includes the test changes look for the 2.5 version to know the new
directive is there, and will require a modification if/when the
directive is back-ported to 2.4 to enable the new tests.

https://bz.apache.org/bugzilla/show_bug.cgi?id=57300

Thanks for your consideration.


Bump. Anyone interested?
--
Paul Spangler
LabVIEW R
National Instruments

Re: svn commit: r1692486 [1/2] - in /httpd/httpd/trunk: docs/log-message-tags/ include/ modules/http2/ modules/ssl/ server/

[Stefan Eissing]

> Am 02.09.2015 um 15:16 schrieb Yann Ylavic :
> 
> I wonder how these handlers interact with mod_proxy_wstunnel which
> needs to handle the Upgrade by itself (after the HTTP header has been
> forwarded to the backend).
> 
> I'm not sure we can upgrade unconditionally really first in the chain.

Hmm. Is the
  ap_hook_handler(core_upgrade_handler,NULL,NULL,APR_HOOK_REALLY_FIRST);
called for requests that mod_proxy_wstunnel makes against a backend?

bytes GmbH
Hafenweg 16, 48155 Münster, Germany
Phone: +49 251 2807760. Amtsgericht Münster: HRB5782

Re: svn commit: r1692486 [1/2] - in /httpd/httpd/trunk: docs/log-message-tags/ include/ modules/http2/ modules/ssl/ server/

[Eric Covener]

On Wed, Sep 2, 2015 at 10:01 AM, Stefan Eissing
 wrote:
> Hmm. Is the
>   ap_hook_handler(core_upgrade_handler,NULL,NULL,APR_HOOK_REALLY_FIRST);
> called for requests that mod_proxy_wstunnel makes against a backend?

yes, the guts of mod_proxy is just a handler, so this would run before that.

Re: Balancers (file based SHMs) and restart issue (Windows only?)

[Yann Ylavic]

On Wed, Sep 2, 2015 at 11:48 AM, Plüm, Rüdiger, Vodafone Group
 wrote:
>
> From: Yann Ylavic [mailto:ylavic@gmail.com]
>>
>> BTW, even on Unixes there is a race on these files between the main
>> process and the children, or the children themselves from different
>> "generation".
>
> Really? I would think that these are files with the same name but different 
> inodes and hence different.

Yes, but I was thinking of eg. an old-generation child removing the
file between shm_create (post_config's slotmem_create) and shm_attach
(child_init's slotmem_attach) from the new-generation.

Re: Balancers (file based SHMs) and restart issue (Windows only?)

[Yann Ylavic]

On Wed, Sep 2, 2015 at 12:32 PM, Yann Ylavic  wrote:
>
> How about the generation in the filename? Windows only?

Or maybe an mktemp() based file for each generation (which has also
the advantage to be implicitely DELONCLOSE).

RE: Balancers (file based SHMs) and restart issue (Windows only?)

[Plüm , Rüdiger , Vodafone Group]

> -Original Message-
> From: Yann Ylavic [mailto:ylavic@gmail.com]
> Sent: Mittwoch, 2. September 2015 11:08
> To: dev@httpd.apache.org
> Subject: Balancers (file based SHMs) and restart issue (Windows only?)
> 
> Re PR 58024.
> 
> AIUI, balancers (and members) SHM slots are destroyed (and the
> underlying base file removed) with pconf before restarting and then
> re-created after (according to the new configuration, eg. number of
> balancers/members, be it the same or not).
> Persisted slots are saved in their own file (with the .persit suffix),
> and reused (copied) only if the configuration did not change in
> between.
> (@Jim, I see now how this is better than attaching ;)
> 
> This works well on Unixes, but on Windows files can't be removed
> (unlinked) while any process/thread hold an HANDLE on it, hence until
> all the children have exited...
> Thus, since the SHM files still exist on restart, the balancer code
> tries to attach them instead of re-creating, and issues the checks on
> the existing size to fit the reloaded configuration, and fail should
> should any balancer/member be added or removed => PR 58024.
> 
> BTW, even on Unixes there is a race on these files between the main
> process and the children, or the children themselves from different
> "generation".

Really? I would think that these are files with the same name but different 
inodes and hence different.

Regards

Rüdiger

Re: Balancers (file based SHMs) and restart issue (Windows only?)

[Yann Ylavic]

On Wed, Sep 2, 2015 at 12:14 PM, Yann Ylavic  wrote:
> On Wed, Sep 2, 2015 at 11:48 AM, Plüm, Rüdiger, Vodafone Group
>  wrote:
>>
>> From: Yann Ylavic [mailto:ylavic@gmail.com]
>>>
>>> BTW, even on Unixes there is a race on these files between the main
>>> process and the children, or the children themselves from different
>>> "generation".
>>
>> Really? I would think that these are files with the same name but different 
>> inodes and hence different.
>
> Yes, but I was thinking of eg. an old-generation child removing the
> file between shm_create (post_config's slotmem_create) and shm_attach
> (child_init's slotmem_attach) from the new-generation.

Hm, that seems to be guarded by the "globallistmem" cache in mod_slotmem_shm...
So I am probably wrong about this race.

On Windows, "globallistmem" is not inherited (forked), hence the
shm_attach() path is taken, so we can't use anonymous SHMs anyway...

How about the generation in the filename? Windows only?

Re: svn commit: r1692486 [1/2] - in /httpd/httpd/trunk: docs/log-message-tags/ include/ modules/http2/ modules/ssl/ server/

[Yann Ylavic]

On Fri, Jul 24, 2015 at 2:09 PM,   wrote:
> Author: icing
> Date: Fri Jul 24 12:09:44 2015
> New Revision: 1692486
>
> URL: http://svn.apache.org/r1692486
> Log:
> new Protocols directive and core API changes to enable protocol switching on 
> HTTP Upgrade or ALPN, implemented in mod_ssl and mod_h2
>
[]
> Modified:
[]
> httpd/httpd/trunk/server/core.c
[]
> ==
> --- httpd/httpd/trunk/server/core.c (original)
> +++ httpd/httpd/trunk/server/core.c Fri Jul 24 12:09:44 2015
[]
> @@ -5226,6 +5253,73 @@ static void core_dump_config(apr_pool_t
[]
> +static int core_upgrade_handler(request_rec *r)
> +{
> +conn_rec *c = r->connection;
> +const char *upgrade = apr_table_get(r->headers_in, "Upgrade");
> +
> +if (upgrade && *upgrade) {
> +const char *conn = apr_table_get(r->headers_in, "Connection");
> +if (ap_find_token(r->pool, conn, "upgrade")) {
> +apr_array_header_t *offers = NULL;
> +const char *err;
> +
> +err = ap_parse_token_list_strict(r->pool, upgrade, , 0);
> +if (err) {
> +ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, APLOGNO(02910)
> +  "parsing Upgrade header: %s", err);
> +return DECLINED;
> +}
> +
> +if (offers && offers->nelts > 0) {
> +const char *protocol = ap_select_protocol(c, r, r->server,
> +  offers);
> +if (strcmp(protocol, ap_run_protocol_get(c))) {
> +ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, 
> APLOGNO(02909)
> +  "Upgrade selects '%s'", protocol);
> +/* Let the client know what we are upgrading to. */
> +apr_table_clear(r->headers_out);
> +apr_table_setn(r->headers_out, "Upgrade", protocol);
> +apr_table_setn(r->headers_out, "Connection", "Upgrade");
> +
> +r->status = HTTP_SWITCHING_PROTOCOLS;
> +r->status_line = ap_get_status_line(r->status);
> +ap_send_interim_response(r, 1);
> +
> +ap_switch_protocol(c, r, r->server, protocol);
> +
> +/* make sure httpd closes the connection after this */
> +c->keepalive = AP_CONN_CLOSE;
> +ap_lingering_close(c);
> +
> +if (c->sbh) {
> +ap_update_child_status_from_conn(c->sbh,
> + SERVER_CLOSING, c);
> +}
> +
> +return DONE;
> +}
> +}
> +}
> +}
> +
> +return DECLINED;
> +}
> +
> +static int core_upgrade_storage(request_rec *r)
> +{
> +if ((r->method_number == M_OPTIONS) && r->uri && (r->uri[0] == '*') &&
> +(r->uri[1] == '\0')) {
> +return core_upgrade_handler(r);
> +}
> +return DECLINED;
> +}
> +
>  static void register_hooks(apr_pool_t *p)
>  {
>  errorlog_hash = apr_hash_make(p);
> @@ -5248,10 +5342,12 @@ static void register_hooks(apr_pool_t *p
[]
> +
> ap_hook_map_to_storage(core_upgrade_storage,NULL,NULL,APR_HOOK_REALLY_FIRST);
[]
> +ap_hook_handler(core_upgrade_handler,NULL,NULL,APR_HOOK_REALLY_FIRST);

I wonder how these handlers interact with mod_proxy_wstunnel which
needs to handle the Upgrade by itself (after the HTTP header has been
forwarded to the backend).

I'm not sure we can upgrade unconditionally really first in the chain.

proposed backport of mod_h2

[Stefan Eissing]

As in r1700829. Regression tests run (after small change on apache version 
check in h2.t) on OS X for me.

Cheers,

  Stefan

bytes GmbH
Hafenweg 16, 48155 Münster, Germany
Phone: +49 251 2807760. Amtsgericht Münster: HRB5782

Re: svn commit: r1692486 [1/2] - in /httpd/httpd/trunk: docs/log-message-tags/ include/ modules/http2/ modules/ssl/ server/

[Yann Ylavic]

On Wed, Sep 2, 2015 at 4:04 PM, Eric Covener  wrote:
> On Wed, Sep 2, 2015 at 10:01 AM, Stefan Eissing
>  wrote:
>> Hmm. Is the
>>   ap_hook_handler(core_upgrade_handler,NULL,NULL,APR_HOOK_REALLY_FIRST);
>> called for requests that mod_proxy_wstunnel makes against a backend?
>
> yes, the guts of mod_proxy is just a handler, so this would run before that.

Maybe core_upgrade_handler could check if the Upgrade's protocol is in
Protocols, and do nothing otherwise...

Re: svn commit: r1692486 [1/2] - in /httpd/httpd/trunk: docs/log-message-tags/ include/ modules/http2/ modules/ssl/ server/

[Stefan Eissing]

> Am 02.09.2015 um 16:12 schrieb Eric Covener :
> 
> On Wed, Sep 2, 2015 at 10:09 AM, Stefan Eissing
>  wrote:
>> But would it have an "Upgrade: + Connection:" header? I might be wrong, but 
>> I thought mod_proxy_wstunnel adds those headers before sending the request 
>> to a backend. So the core_ugprade_handler would not see those.
> 
> 
> That module is not http-to-ws gateway, it's ws-to-ws, so the
> normal/frontend request will have all of that stuff from the client.

But *unless* there is someone inside httpd that implements WebSocket *and* 
participates in protocol negotiations *and* is willing to directly upgrade to / 
ALPN select the WebSocket protocol, the protocol switching will never trigger.

So, if we imagine some module wanting to implement WebSockets directly in 
httpd, what would it need to do?

- If the client sends http/1.1 requests with "Upgrade: WebSocket", the module 
could propose WebSocket and let the server do the switch. It then gets control 
of the connection. Either it answers the WebSocket directly or uses a backend 
connection somewhere else, this will not be visible to the client.
- If the client sends WebSocket in an ALPN negotiation and there is a module 
handling that for the SNI server given, it can take over the connection right 
away.
- If the client expects some WebSocket on a connection *without* any HTTP/1.1 
Upgrade or TLS ALPN magic, protocol negotiation does not trigger and all is as 
before.

As a last resort, the server can always be configured with a "Protocols" 
without "WebSocket" in it and thus our new negotiation would do nothing.

Perhaps you could make a more detailed description of where the implemented 
Protocols cause a problem?


bytes GmbH
Hafenweg 16, 48155 Münster, Germany
Phone: +49 251 2807760. Amtsgericht Münster: HRB5782

Re: svn commit: r1692486 [1/2] - in /httpd/httpd/trunk: docs/log-message-tags/ include/ modules/http2/ modules/ssl/ server/

[Eric Covener]

On Wed, Sep 2, 2015 at 10:09 AM, Stefan Eissing
 wrote:
> But would it have an "Upgrade: + Connection:" header? I might be wrong, but I 
> thought mod_proxy_wstunnel adds those headers before sending the request to a 
> backend. So the core_ugprade_handler would not see those.


That module is not http-to-ws gateway, it's ws-to-ws, so the
normal/frontend request will have all of that stuff from the client.

Re: svn commit: r1692486 [1/2] - in /httpd/httpd/trunk: docs/log-message-tags/ include/ modules/http2/ modules/ssl/ server/

[Stefan Eissing]

> Am 02.09.2015 um 16:19 schrieb Yann Ylavic :
> 
> On Wed, Sep 2, 2015 at 4:04 PM, Eric Covener  wrote:
>> On Wed, Sep 2, 2015 at 10:01 AM, Stefan Eissing
>>  wrote:
>>> Hmm. Is the
>>>  ap_hook_handler(core_upgrade_handler,NULL,NULL,APR_HOOK_REALLY_FIRST);
>>> called for requests that mod_proxy_wstunnel makes against a backend?
>> 
>> yes, the guts of mod_proxy is just a handler, so this would run before that.
> 
> Maybe core_upgrade_handler could check if the Upgrade's protocol is in
> Protocols, and do nothing otherwise...

It calls ap_select_protocol(...) and that one allows only selections from the 
configured "Protocols" list.

//Stefan

bytes GmbH
Hafenweg 16, 48155 Münster, Germany
Phone: +49 251 2807760. Amtsgericht Münster: HRB5782