Re: svn commit: r1311569 - in /httpd/mod_fcgid/trunk: CHANGES-FCGID docs/manual/mod/mod_fcgid.xml modules/fcgid/fcgid_conf.c modules/fcgid/fcgid_conf.h modules/fcgid/fcgid_pm_win.c modules/fcgid/fcgid
On 4/10/2012 8:27 PM, Jeff Trawick wrote:
> On Tue, Apr 10, 2012 at 11:55 AM, William A. Rowe Jr.
> wrote:
>> On 4/10/2012 10:31 AM, Jeff Trawick wrote:
>>> On Tue, Apr 10, 2012 at 12:05 AM, wrote:
+/* Cleanup the Job object if present */
+conf = ap_get_module_config(((server_rec*)server)->module_config,
+&fcgid_module);
+
+if (conf != NULL && conf->hJobObjectForAutoCleanup != NULL) {
+CloseHandle(conf->hJobObjectForAutoCleanup);
+}
+
>>>
>>> Isn't it more idiomatic to register a cleanup for the job object
>>> rather than explicitly checking for whether or not it exists in
>>> different code?
>>
>> +1
>
> I haven't touched that. After possibly wasting time hacking the error
> reporting/handling for when the job object is created, I wonder if
> this is even the right place to create the job object and potentially
> register a cleanup. Why not in a post-config hook? Also, is this
> really needed in parent AND child?
The windows logic needs a lot more thought in relation to the parent and
child, where this pool of fcgid workers is created, how they are released.
But as job objects, they will be gone as the parent dies, so I believe
the whole theory is fundamentally sound. Cosmetics like this do deserve
deeper consideration, but I think it's ready for release as is.
Re: svn commit: r1311569 - in /httpd/mod_fcgid/trunk: CHANGES-FCGID docs/manual/mod/mod_fcgid.xml modules/fcgid/fcgid_conf.c modules/fcgid/fcgid_conf.h modules/fcgid/fcgid_pm_win.c modules/fcgid/fcgid
On Tue, Apr 10, 2012 at 11:55 AM, William A. Rowe Jr.
wrote:
> On 4/10/2012 10:31 AM, Jeff Trawick wrote:
>> On Tue, Apr 10, 2012 at 12:05 AM, wrote:
>>> Author: wrowe
>>> Date: Tue Apr 10 04:05:23 2012
>>> New Revision: 1311569
>>>
>>> URL: http://svn.apache.org/viewvc?rev=1311569&view=rev
>>> Log:
>>> Introduce FcgidWin32PreventOrphans directive on Windows to use OS
>>> Job Control Objects to terminate all running fcgi's when the worker
>>> process has been abruptly terminated.
>>>
>>> [wrowe: I've changed the volume level on some error levels, moved
>>> the rv into the error log schema, and moved all commentary over to
>>> the actual conf assignment function. With the exception of style
>>> cleanup, this remains almost entirely Thangaraj's creation. Also
>>> have added simple docs.]
>>>
>>> PR: 51078
>>> Submitted by: Thangaraj AntonyCrouse
>>
>> Is anyone opposed to changing the directive from _NO_ARGS to _FLAG?
>
> Make it so.
>
>> (a couple of more comments are below)
>>
>> I'm happy to do the tweaks after confirmation.
>
> Thanks!
>
>>>
>>> + /* Cleanup the Job object if present */
>>> + conf = ap_get_module_config(((server_rec*)server)->module_config,
>>> + &fcgid_module);
>>> +
>>> + if (conf != NULL && conf->hJobObjectForAutoCleanup != NULL) {
>>> + CloseHandle(conf->hJobObjectForAutoCleanup);
>>> + }
>>> +
>>
>> Isn't it more idiomatic to register a cleanup for the job object
>> rather than explicitly checking for whether or not it exists in
>> different code?
>
> +1
I haven't touched that. After possibly wasting time hacking the error
reporting/handling for when the job object is created, I wonder if
this is even the right place to create the job object and potentially
register a cleanup. Why not in a post-config hook? Also, is this
really needed in parent AND child?
>
>>> --- httpd/mod_fcgid/trunk/modules/fcgid/fcgid_proc_win.c (original)
>>> +++ httpd/mod_fcgid/trunk/modules/fcgid/fcgid_proc_win.c Tue Apr 10
>>> 04:05:23 2012
>>> @@ -63,6 +63,7 @@ apr_status_t proc_spawn_process(const ch
>>> {
>>> HANDLE *finish_event, listen_handle;
>>> SECURITY_ATTRIBUTES SecurityAttributes;
>>> + fcgid_server_conf *sconf;
>>> apr_procattr_t *proc_attr;
>>> apr_status_t rv;
>>> apr_file_t *file;
>>> @@ -177,9 +178,31 @@ apr_status_t proc_spawn_process(const ch
>>> if (rv != APR_SUCCESS) {
>>> ap_log_error(APLOG_MARK, APLOG_ERR, rv, procinfo->main_server,
>>> "mod_fcgid: can't run %s", wargv[0]);
>>> + return rv;
>>> }
>
> We leave the new return rv, above.
>
>>> - return rv;
>>> + /* FcgidWin32PreventOrphans feature */
>>> + sconf = ap_get_module_config(procinfo->main_server->module_config,
>>> + &fcgid_module);
>>> + if (sconf == NULL) {
>>> + ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, procinfo->main_server,
>>> + "mod_fcgid: missing server configuration record");
>>> + return APR_SUCCESS;
>>> + }
>>
>> Let's just crash here if sconf is NULL. Agreed?
>
> +1
>
--
Born in Roswell... married an alien...
Re: Scope of ProxyPreserveHost
Thanks, Kaspar, for the info. That's exactly what I had in mind. And thanks for this new great feature in 2.3/4! Jie * Kaspar Brand wrote: > Date: Tue, 10 Apr 2012 18:38:02 +0200 > From: Kaspar Brand > To: dev@httpd.apache.org > Subject: Re: Scope of ProxyPreserveHost > > On 10.04.2012 12:38, Tom Evans wrote: > > On Tue, Apr 10, 2012 at 1:38 AM, Jie Gao wrote: > >> Hi All > >> > >> Would it be possible to expand the scope of ProxyPreserveHost to > >> "Location"? > >> > >> Regards, > >> > >> > >> Jie > >> > > > > I don't understand; ProxyPreserveHost affects the proxying apache > > server, and controls whether it rewrites the 'Host' header in the > > request with the selected backend's hostname or not. > > > > 'Location' on the other hand is a response header. mod_proxy already > > has a directive to rewrite response headers like 'Location', see > > ProxyPassReverse. > > s/"Location"//, presumably. > > Per-dir support for ProxyPreserveHost was added in 2.3.3, actually: > https://issues.apache.org/bugzilla/show_bug.cgi?id=34901. > > Kaspar
mod_fcgid graceful restarts
Hi All, Currently graceful restart while using mod_fcgid just kill all subprocesses, this is not sefe for applications and slows down reloads. John Lightsey provided a patch to make real graceful restarts on mod_fcgid, now graceful part is separated as requested in the bug report https://issues.apache.org/bugzilla/show_bug.cgi?id=48769 I am running with this patch on over 10 machines doing daily 24+ graceful restarts, so far without any issues (reloads are faster, and there are no orphan left behind.) Regards, Michal Grzedzicki
Re: CHANGES-FCGID is incorrect
Bill, Jeff, I agree 2.3.7 needs to be released for the stated reasons, I was +1 to that back a few weeks ago. On 4/10/2012 9:11 AM, Jeff Trawick wrote: On Tue, Apr 10, 2012 at 12:35 AM, William A. Rowe Jr. wrote: On 4/9/2012 11:30 PM, William A. Rowe Jr. wrote: The patch does have value to a limited number of applications. I even went as far as to put caviats in the docs, and a see-docs note to the directive cmd commentary. I hope it dissuades the casual user from throwing it on there unless they know it won't corrupt their app and know it solves their bug. Please reconsider your veto. I agree that it -should not- occur in any real world scenario. But there are unreal scenarios of server configs and third party modules which spend minutes, not seconds, tearing down on shutdown. Those are the bugs. But users just want some help and I think this patch is some help in rare cases. Disabled by default and won't be used by much of anyone, we hope. I'll revert (yet again) if you really want to make a case that we should never support this, even as a workaround. As far as my specific veto, I wouldn't call it a veto but more a concern as it's untested code vs. 50309 which is well tested, I'm trusting your knowledge here. Bill, your caveats in the docs about cleanup, being experimental and off by default is good enough for me. I should be able to build and do a little testing within the next 48 hours. I'll have to look over the 50120 &51520 statement of mine hopefully within same time frame. I may have mixed up the number quit easily! Gregg, and Jeff and company, if this is reviewed and everyone is happy with an end result, I'd like to ensure 2.4.2 compatibility and get tagging. Gregg? My gut feel is that the feature will help in some real world circumstances, but I need to stare at Windows-specific code for a while to really understand the problem (where third-party modules can intervene, where a timeout comes into play, etc.). At first glance it seems to simply be the case when apr_proc_kill(SIGKILL) doesn't work for whatever reason (third-party code, but not the usual code we worry about). We seem well overdue for a release. There is a group of issues with spaces in command lines, spaces in file names that need to be resolved for the typical windows scenario (and obviously also unix, just more unusual there). I think we are best off calling 2.3.7 about baked right now, tagging, reviewing and releasing, and calling 2.3.next the flavor which fixes these command/file string argument quirks. +1
Re: Scope of ProxyPreserveHost
On 10.04.2012 12:38, Tom Evans wrote: > On Tue, Apr 10, 2012 at 1:38 AM, Jie Gao wrote: >> Hi All >> >> Would it be possible to expand the scope of ProxyPreserveHost to "Location"? >> >> Regards, >> >> >> Jie >> > > I don't understand; ProxyPreserveHost affects the proxying apache > server, and controls whether it rewrites the 'Host' header in the > request with the selected backend's hostname or not. > > 'Location' on the other hand is a response header. mod_proxy already > has a directive to rewrite response headers like 'Location', see > ProxyPassReverse. s/"Location"//, presumably. Per-dir support for ProxyPreserveHost was added in 2.3.3, actually: https://issues.apache.org/bugzilla/show_bug.cgi?id=34901. Kaspar
Re: CHANGES-FCGID is incorrect
On Tue, Apr 10, 2012 at 12:35 AM, William A. Rowe Jr. wrote: > On 4/9/2012 11:30 PM, William A. Rowe Jr. wrote: >> >> The patch does have value to a limited number of applications. I even went >> as far as to put caviats in the docs, and a see-docs note to the directive >> cmd commentary. I hope it dissuades the casual user from throwing it on >> there >> unless they know it won't corrupt their app and know it solves their bug. >> >> Please reconsider your veto. I agree that it -should not- occur in any >> real world scenario. But there are unreal scenarios of server configs >> and third party modules which spend minutes, not seconds, tearing down on >> shutdown. Those are the bugs. But users just want some help and I think >> this patch is some help in rare cases. Disabled by default and won't be >> used by much of anyone, we hope. I'll revert (yet again) if you really >> want to make a case that we should never support this, even as a workaround. > > Gregg, and Jeff and company, if this is reviewed and everyone is happy with > an end result, I'd like to ensure 2.4.2 compatibility and get tagging. Gregg? My gut feel is that the feature will help in some real world circumstances, but I need to stare at Windows-specific code for a while to really understand the problem (where third-party modules can intervene, where a timeout comes into play, etc.). At first glance it seems to simply be the case when apr_proc_kill(SIGKILL) doesn't work for whatever reason (third-party code, but not the usual code we worry about). > > We seem well overdue for a release. > > There is a group of issues with spaces in command lines, spaces in file names > that need to be resolved for the typical windows scenario (and obviously also > unix, just more unusual there). I think we are best off calling 2.3.7 about > baked right now, tagging, reviewing and releasing, and calling 2.3.next the > flavor which fixes these command/file string argument quirks. +1
Re: svn commit: r1311569 - in /httpd/mod_fcgid/trunk: CHANGES-FCGID docs/manual/mod/mod_fcgid.xml modules/fcgid/fcgid_conf.c modules/fcgid/fcgid_conf.h modules/fcgid/fcgid_pm_win.c modules/fcgid/fcgid
On 4/10/2012 10:31 AM, Jeff Trawick wrote:
> On Tue, Apr 10, 2012 at 12:05 AM, wrote:
>> Author: wrowe
>> Date: Tue Apr 10 04:05:23 2012
>> New Revision: 1311569
>>
>> URL: http://svn.apache.org/viewvc?rev=1311569&view=rev
>> Log:
>> Introduce FcgidWin32PreventOrphans directive on Windows to use OS
>> Job Control Objects to terminate all running fcgi's when the worker
>> process has been abruptly terminated.
>>
>> [wrowe: I've changed the volume level on some error levels, moved
>> the rv into the error log schema, and moved all commentary over to
>> the actual conf assignment function. With the exception of style
>> cleanup, this remains almost entirely Thangaraj's creation. Also
>> have added simple docs.]
>>
>> PR: 51078
>> Submitted by: Thangaraj AntonyCrouse
>
> Is anyone opposed to changing the directive from _NO_ARGS to _FLAG?
Make it so.
> (a couple of more comments are below)
>
> I'm happy to do the tweaks after confirmation.
Thanks!
>>
>> +/* Cleanup the Job object if present */
>> +conf = ap_get_module_config(((server_rec*)server)->module_config,
>> +&fcgid_module);
>> +
>> +if (conf != NULL && conf->hJobObjectForAutoCleanup != NULL) {
>> +CloseHandle(conf->hJobObjectForAutoCleanup);
>> +}
>> +
>
> Isn't it more idiomatic to register a cleanup for the job object
> rather than explicitly checking for whether or not it exists in
> different code?
+1
>> --- httpd/mod_fcgid/trunk/modules/fcgid/fcgid_proc_win.c (original)
>> +++ httpd/mod_fcgid/trunk/modules/fcgid/fcgid_proc_win.c Tue Apr 10 04:05:23
>> 2012
>> @@ -63,6 +63,7 @@ apr_status_t proc_spawn_process(const ch
>> {
>> HANDLE *finish_event, listen_handle;
>> SECURITY_ATTRIBUTES SecurityAttributes;
>> +fcgid_server_conf *sconf;
>> apr_procattr_t *proc_attr;
>> apr_status_t rv;
>> apr_file_t *file;
>> @@ -177,9 +178,31 @@ apr_status_t proc_spawn_process(const ch
>> if (rv != APR_SUCCESS) {
>> ap_log_error(APLOG_MARK, APLOG_ERR, rv, procinfo->main_server,
>> "mod_fcgid: can't run %s", wargv[0]);
>> +return rv;
>> }
We leave the new return rv, above.
>> -return rv;
>> +/* FcgidWin32PreventOrphans feature */
>> +sconf = ap_get_module_config(procinfo->main_server->module_config,
>> + &fcgid_module);
>> +if (sconf == NULL) {
>> +ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, procinfo->main_server,
>> + "mod_fcgid: missing server configuration record");
>> +return APR_SUCCESS;
>> +}
>
> Let's just crash here if sconf is NULL. Agreed?
+1
Re: svn commit: r1311569 - in /httpd/mod_fcgid/trunk: CHANGES-FCGID docs/manual/mod/mod_fcgid.xml modules/fcgid/fcgid_conf.c modules/fcgid/fcgid_conf.h modules/fcgid/fcgid_pm_win.c modules/fcgid/fcgid
On Tue, Apr 10, 2012 at 12:05 AM, wrote: > Author: wrowe > Date: Tue Apr 10 04:05:23 2012 > New Revision: 1311569 > > URL: http://svn.apache.org/viewvc?rev=1311569&view=rev > Log: > Introduce FcgidWin32PreventOrphans directive on Windows to use OS > Job Control Objects to terminate all running fcgi's when the worker > process has been abruptly terminated. > > [wrowe: I've changed the volume level on some error levels, moved > the rv into the error log schema, and moved all commentary over to > the actual conf assignment function. With the exception of style > cleanup, this remains almost entirely Thangaraj's creation. Also > have added simple docs.] > > PR: 51078 > Submitted by: Thangaraj AntonyCrouse Is anyone opposed to changing the directive from _NO_ARGS to _FLAG? (a couple of more comments are below) I'm happy to do the tweaks after confirmation. > Modified: > httpd/mod_fcgid/trunk/CHANGES-FCGID > httpd/mod_fcgid/trunk/docs/manual/mod/mod_fcgid.xml > httpd/mod_fcgid/trunk/modules/fcgid/fcgid_conf.c > httpd/mod_fcgid/trunk/modules/fcgid/fcgid_conf.h > httpd/mod_fcgid/trunk/modules/fcgid/fcgid_pm_win.c > httpd/mod_fcgid/trunk/modules/fcgid/fcgid_proc_win.c > httpd/mod_fcgid/trunk/modules/fcgid/mod_fcgid.c > > Modified: httpd/mod_fcgid/trunk/CHANGES-FCGID > URL: > http://svn.apache.org/viewvc/httpd/mod_fcgid/trunk/CHANGES-FCGID?rev=1311569&r1=1311568&r2=1311569&view=diff > == > --- httpd/mod_fcgid/trunk/CHANGES-FCGID [utf8] (original) > +++ httpd/mod_fcgid/trunk/CHANGES-FCGID [utf8] Tue Apr 10 04:05:23 2012 > @@ -1,6 +1,11 @@ > -*- coding: utf-8 -*- > Changes with mod_fcgid 2.3.7 > > + *) Introduce FcgidWin32PreventOrphans directive on Windows to use OS > + Job Control Objects to terminate all running fcgi's when the worker > + process has been abruptly terminated. PR: 51078 > + [Thangaraj AntonyCrouse ] > + > *) Periodically clean out the brigades which are pulling in the request > body for handoff to the fcgid child. PR: 51749 > [Dominic Benson ] > > Modified: httpd/mod_fcgid/trunk/docs/manual/mod/mod_fcgid.xml > URL: > http://svn.apache.org/viewvc/httpd/mod_fcgid/trunk/docs/manual/mod/mod_fcgid.xml?rev=1311569&r1=1311568&r2=1311569&view=diff > == > --- httpd/mod_fcgid/trunk/docs/manual/mod/mod_fcgid.xml (original) > +++ httpd/mod_fcgid/trunk/docs/manual/mod/mod_fcgid.xml Tue Apr 10 04:05:23 > 2012 > @@ -1124,6 +1124,22 @@ > > > > + FcgidWin32PreventOrphans > + Job Control orphan prevention for fcgi > workers. > + FcgidWin32PreventOrphans > + [disabled] > + server config > + > + Uses Job Control Objects on Windows, only, to enforce shutdown of > all > + fcgi processes created by the httpd worker when the httpd worker has > been > + terminated. Processes terminated in this way do not have the > opportunity > + to clean up gracefully, complete pending disk writes, or similar > closure > + transactions, therefore this behavior is experimental and disabled, by > + default. > + > + > + > + > FcgidWrapper > The CGI wrapper setting > FcgidWrapper command [ suffix ] [ virtual > ] > > Modified: httpd/mod_fcgid/trunk/modules/fcgid/fcgid_conf.c > URL: > http://svn.apache.org/viewvc/httpd/mod_fcgid/trunk/modules/fcgid/fcgid_conf.c?rev=1311569&r1=1311568&r2=1311569&view=diff > == > --- httpd/mod_fcgid/trunk/modules/fcgid/fcgid_conf.c (original) > +++ httpd/mod_fcgid/trunk/modules/fcgid/fcgid_conf.c Tue Apr 10 04:05:23 2012 > @@ -749,6 +749,71 @@ const char *set_access_authoritative(cmd > return NULL; > } > > + > +#ifdef WIN32 > +/* FcgidWin32PreventOrphans > + * > + * When Apache process gets recycled or shutdown abruptly, CGI processes > + * spawned by mod_fcgid will get orphaned. Orphaning happens mostly when > + * Apache worker threads take more than 30 seconds to exit gracefully. > + * > + * Apache when run as windows service during shutdown/restart of service > + * process (master/parent) will terminate child httpd process within 30 > + * seconds (refer \server\mpm\winnt\mpm_winnt.c:master_main() > + * int timeout = 3; ~line#1142), therefore if Apache worker threads > + * are too busy to react to Master's graceful exit signal within 30 seconds > + * mod_fcgid cleanup routines will not get invoked (refer child_main() > + * \server\mpm\winnt\child.c: apr_pool_destroy(pchild); ~line#2275) > + * thereby orphaning all mod_fcgid spwaned CGI processes. Therefore we > utilize > + * Win32 JobObjects to clean up child processes automatically so that CGI > + * processes are force-killed by win32 during abnormal mod_fcgid termination. > + * > + */ > +const char *set_win32_prevent_process_
Re: [VOTE] Release Apache httpd 2.4.2 as GA
- Original Message - > The pre-release test tarballs for Apache httpd 2.4.2 can be found > at the usual place: > > http://httpd.apache.org/dev/dist/ > > I'm calling a VOTE on releasing these as Apache httpd 2.4.2 GA. > NOTE: The -deps tarballs are included here *only* to make life > easier for the tester. They will not be, and are not, part > of the official release. > > [x] +1: Good to go > [ ] +0: meh > [ ] -1: Danger Will Robinson. And why. > > Vote will last the normal 72 hrs. > Fedora 16/amd64. apr $latest stable. Works fine with PHP, too. i -- Igor Galić Tel: +43 (0) 664 886 22 883 Mail: i.ga...@brainsware.org URL: http://brainsware.org/ GPG: 6880 4155 74BD FD7C B515 2EA5 4B1D 9E08 A097 C9AE
Re: Scope of ProxyPreserveHost (was: Re: [users@httpd] SNI with apache 2.4.1 reverse proxy)
On Tue, Apr 10, 2012 at 1:38 AM, Jie Gao wrote: > Hi All > > Would it be possible to expand the scope of ProxyPreserveHost to "Location"? > > Regards, > > > Jie > I don't understand; ProxyPreserveHost affects the proxying apache server, and controls whether it rewrites the 'Host' header in the request with the selected backend's hostname or not. 'Location' on the other hand is a response header. mod_proxy already has a directive to rewrite response headers like 'Location', see ProxyPassReverse. Cheers Tom
Re: [users@httpd] SNI with apache 2.4.1 reverse proxy
Hi Igor, Hi Daniel, On Mon, Apr 09, 2012 at 08:56:12AM -, Igor Gali? wrote: > > Then it looks like mod_proxy_http determines the value for > > "proxy-request-hostname" from the remote URL in ProxyPass, but is > > passing on the Host header from the original request. > That would imply ProxyPreserveHost on -- which is off by default > I also don't see it in Micha's paste. Uh, I am very sorry to have wasted your time, but I actually do have ProxyPreserveHost On in my config. It was inbetween some comments and I must have removed it together with them. I have checked and it seems to be the only statement missing from my mail. I have it in there because wordpress has a feature of automatically using the host name from the request in all links in the HTML it generates. Unfortunately, it insists on creating absolute instead of relative links. This is also why I access wordpress inside the VM via HTTPS at all: This way it automatically (or at least with only a very small patch to it's config.php) generates https:// links in its responses when accessed via HTTPS, making the reverse proxy very simple (apart from the SSL bit) and almost transparent. At first I tried to configure the reverse proxy to plain http:// (SSL termination, so to speak) and rewrite all links using mod_proxy_html for performance and because it seemed the straightforward thing to do. But I had various detail problems within wordpress I couldn't solve (with links to uploaded files for example). So I switched to just passing on the original requests as unchanged as possible. As for the SNI bit: So I tell the reverse proxy to access https://www.example.com:12433/ but pass on the Host header unchanged. The wordpress VM's apache 2.2.14 gets upset with this discrepancy and denies to serve the requests. As I perhaps poorly explained in the second part of my mail, I tried to tell the reverse proxy to access https://:12443/ instead but couldn't make it work. A solution might be something like: ProxyPass / https://www.example.com:12443/ no-sni ProxyPassReverse / https://www.example.com:12443/ no-sni , disabling SNI towards the backend server. Or can I tell the 2.2.14 apache inside the VM to ignore the SNI data it sees in the requests? The best solution I can think of would be some switch like ProxyPass / https://www.example.com:12443/ pass-host-as-sni ProxyPassReverse / https://www.example.com:12443/ pass-host-as-sni that makes mod_ssl put the content of the host header into the sni data structures instead of the hostname from the URL used in the ProxyPass(Reverse) configuration itself. This way even name-based virtual hosts should work behind the reverse proxy. -- Thanks for your patience, Micha
