Re: cvs commit: httpd-2.0/server/mpm/winnt mpm_winnt.c
The last several changes should resolve a series of fairly random segfaults
that have been observed at shutdown by Covalent's QA group [I was even
able to reproduce in 'console' mode.] Although I'd love another pair of eyes
on these latest changes, I'll include them in the .34 release so we don't
waste time closing the ensuing bugzilla reports.
Bill
wrowe 02/04/04 15:36:45
Modified:server/mpm/winnt mpm_winnt.c
Log:
Uhmmm... notices only when we are running as a service [I hate strcmp.]
Revision ChangesPath
1.259 +1 -1 httpd-2.0/server/mpm/winnt/mpm_winnt.c
Index: mpm_winnt.c
===
RCS file: /home/cvs/httpd-2.0/server/mpm/winnt/mpm_winnt.c,v
retrieving revision 1.258
retrieving revision 1.259
diff -u -r1.258 -r1.259
--- mpm_winnt.c 4 Apr 2002 23:35:11 - 1.258
+++ mpm_winnt.c 4 Apr 2002 23:36:45 - 1.259
-1850,7 +1850,7
* child a bit of time to exit gracefully. If the time expires,
* the child will be wacked.
*/
-if (strcasecmp(signal_arg, runservice)) {
+if (!strcasecmp(signal_arg, runservice)) {
mpm_service_stopping();
}
/* Signal the child processes to exit */
Re: cvs commit: httpd-2.0/server/mpm/winnt mpm_winnt.c
At 08:48 AM 4/2/2002, you wrote: stoddard02/04/02 06:48:54 Modified:server/mpm/winnt mpm_winnt.c Log: Win32: tweak some messages Revision ChangesPath 1.255 +2 -2 httpd-2.0/server/mpm/winnt/mpm_winnt.c The NOTICE level reversion and message tweaks will definitely make it easier to identify what happened when looking at bug report's log files. I've moved this revision into our APACHE_2_0_34 release, unless someone has any objections. Bill
Re: cvs commit: httpd-2.0/server/mpm/winnt mpm_winnt.c mpm_winnt.h
Now incorporated into APACHE_2_0_34 stoddard02/04/01 10:55:46 Modified:server/mpm/winnt mpm_winnt.c mpm_winnt.h Log: Win32: Move apr_bucket_alloc() to a more reasonable location to fix memory leak. Revision ChangesPath 1.253 +4 -6 httpd-2.0/server/mpm/winnt/mpm_winnt.c 1.36 +1 -0 httpd-2.0/server/mpm/winnt/mpm_winnt.h
Re: cvs commit: httpd-2.0/server/mpm/winnt mpm_winnt.c
[EMAIL PROTECTED] writes:
On 20 Mar 2002 [EMAIL PROTECTED] wrote:
wrowe 02/03/19 20:29:55
Modified:server/mpm/winnt mpm_winnt.c
Log:
When restarting [always graceful on Win32], we don't repeat pre_mpm
(Unix doesn't, we shouldn't either.) [Ryan Bloom]
-if ((parent_pid == my_pid) || one_process) {
+/* ### If non-graceful restarts are ever introduced - we need to rerun
+ * the pre_mpm hook on subsequent non-graceful restarts. But Win32
+ * has only graceful style restarts - and we need this hook to act
+ * the same on Win32 as on Unix.
+ */
+if (!restart ((parent_pid == my_pid) || one_process)) {
/* Set up the scoreboard. */
if (ap_run_pre_mpm(pconf, SB_SHARED) != OK) {
return 1;
While I agree with this patch, you also need to kill the cleanup on the
scoreboard, so that it isn't set to NULL when pconf is cleared.
Actually, the cleanup never runs because it never finds the data in
the pconf pool :) But yes it needs to be dealt with.
I was cleaning this issue up this morning (removing cleanups, removing
logic I added recently to restore the proper running_generation field)
and realized (or was confused into thinking) that we aren't handling
non-graceful restart anymore, in that we don't start back with a fresh
scoreboard (fresh except for the running_generation field). Now,
whatever was in the scoreboard from the previous, whacked generation
of children is still there.
For now I've set aside the cleanup changes until there is some
resolution on how it should be handled (should MPM have to clean up,
should scoreboard-create clean up when it realizes that it doesn't
have to reallocate, etc.).
--
Jeff Trawick | [EMAIL PROTECTED]
Born in Roswell... married an alien...
Re: cvs commit: httpd-2.0/server/mpm/winnt mpm_winnt.c
Jeff Trawick [EMAIL PROTECTED] writes:
[EMAIL PROTECTED] writes:
On 20 Mar 2002 [EMAIL PROTECTED] wrote:
wrowe 02/03/19 20:29:55
Modified:server/mpm/winnt mpm_winnt.c
Log:
When restarting [always graceful on Win32], we don't repeat pre_mpm
(Unix doesn't, we shouldn't either.) [Ryan Bloom]
-if ((parent_pid == my_pid) || one_process) {
+/* ### If non-graceful restarts are ever introduced - we need to rerun
+ * the pre_mpm hook on subsequent non-graceful restarts. But Win32
+ * has only graceful style restarts - and we need this hook to act
+ * the same on Win32 as on Unix.
+ */
+if (!restart ((parent_pid == my_pid) || one_process)) {
/* Set up the scoreboard. */
if (ap_run_pre_mpm(pconf, SB_SHARED) != OK) {
return 1;
While I agree with this patch, you also need to kill the cleanup on the
scoreboard, so that it isn't set to NULL when pconf is cleared.
Actually, the cleanup never runs because it never finds the data in
the pconf pool :) But yes it needs to be dealt with.
wrong... let me restate... we currently have logic to kill the
cleanup for graceful restart but that call to cleanup_kill is bogus in
that 1) it searches the wrong pool and 2) if OtherBill's changes to
keep using the same scoreboard forever and ever then we don't need to
kill the cleanup anyway
--
Jeff Trawick | [EMAIL PROTECTED]
Born in Roswell... married an alien...
RE: cvs commit: httpd-2.0/server/mpm/winnt mpm_winnt.c
And, you need to find some way to pass the scoreboard back to the child process because you are about to start a new one. What are you talking about, we killed Kenny :0/ This next generation we have new offspring. yeah, but the new child process needs the same information as the old one did. Won't you need to pass the scoreboard to the new child, and isn't that done by the pre_mpm hook? I could very easily be wrong about which hook does that work, but the last time I looked, I thought it was pre_mpm. Ryan
RE: cvs commit: httpd-2.0/server/mpm/winnt mpm_winnt.c
At 08:41 AM 3/20/2002, you wrote: yeah, but the new child process needs the same information as the old one did. Won't you need to pass the scoreboard to the new child, and isn't that done by the pre_mpm hook? I could very easily be wrong about which hook does that work, but the last time I looked, I thought it was pre_mpm. No, pre_mpm doesn't pass anything from parent-child [that would be broken already by the pre_mpm-only-once patch.] The create_process logic within mpm_winnt calls two passing fns, one of handles in general, one for the listeners. It's invoked every time we create a child. We have two matching fn's called by child_main() that slurp those general handlers and listeners, respectively. Whatever the ap_scoreboard_shm's os_handle, we pass it. We call ap_scoreboard_init, not to init the score, but to init the local storage, e.g. array pointers into the scoreboard memory. The memory isn't passed to the child, the handle to this memory is passed; the child reopens the scoreboard by its handle. Bill
RE: cvs commit: httpd-2.0/server/mpm/winnt mpm_winnt.c
Cool, thanks for the info. This is much clearer now. Ryan -- Ryan Bloom [EMAIL PROTECTED] 645 Howard St. [EMAIL PROTECTED] San Francisco, CA -Original Message- From: William A. Rowe, Jr. [mailto:[EMAIL PROTECTED]] Sent: Wednesday, March 20, 2002 8:28 AM To: [EMAIL PROTECTED] Subject: RE: cvs commit: httpd-2.0/server/mpm/winnt mpm_winnt.c At 08:41 AM 3/20/2002, you wrote: yeah, but the new child process needs the same information as the old one did. Won't you need to pass the scoreboard to the new child, and isn't that done by the pre_mpm hook? I could very easily be wrong about which hook does that work, but the last time I looked, I thought it was pre_mpm. No, pre_mpm doesn't pass anything from parent-child [that would be broken already by the pre_mpm-only-once patch.] The create_process logic within mpm_winnt calls two passing fns, one of handles in general, one for the listeners. It's invoked every time we create a child. We have two matching fn's called by child_main() that slurp those general handlers and listeners, respectively. Whatever the ap_scoreboard_shm's os_handle, we pass it. We call ap_scoreboard_init, not to init the score, but to init the local storage, e.g. array pointers into the scoreboard memory. The memory isn't passed to the child, the handle to this memory is passed; the child reopens the scoreboard by its handle. Bill
Re: cvs commit: httpd-2.0/server/mpm/winnt mpm_winnt.c
On 20 Mar 2002 [EMAIL PROTECTED] wrote:
wrowe 02/03/19 20:29:55
Modified:server/mpm/winnt mpm_winnt.c
Log:
When restarting [always graceful on Win32], we don't repeat pre_mpm
(Unix doesn't, we shouldn't either.) [Ryan Bloom]
-if ((parent_pid == my_pid) || one_process) {
+/* ### If non-graceful restarts are ever introduced - we need to rerun
+ * the pre_mpm hook on subsequent non-graceful restarts. But Win32
+ * has only graceful style restarts - and we need this hook to act
+ * the same on Win32 as on Unix.
+ */
+if (!restart ((parent_pid == my_pid) || one_process)) {
/* Set up the scoreboard. */
if (ap_run_pre_mpm(pconf, SB_SHARED) != OK) {
return 1;
While I agree with this patch, you also need to kill the cleanup on the
scoreboard, so that it isn't set to NULL when pconf is cleared. And, you
need to find some way to pass the scoreboard back to the child process
because you are about to start a new one.
Ryan
___
Ryan Bloom [EMAIL PROTECTED]
550 Jean St
Oakland CA 94610
---
Re: cvs commit: httpd-2.0/server/mpm/winnt mpm_winnt.c
At 11:31 PM 3/19/2002, you wrote:
On 20 Mar 2002 [EMAIL PROTECTED] wrote:
+if (!restart ((parent_pid == my_pid) || one_process)) {
/* Set up the scoreboard. */
if (ap_run_pre_mpm(pconf, SB_SHARED) != OK) {
return 1;
While I agree with this patch, you also need to kill the cleanup on the
scoreboard, so that it isn't set to NULL when pconf is cleared.
No you do not kill the cleanup - you rescope it. See the next patch.
Really, lovely, let's off and leave a dangling object that fell out of scope
with no cleanup. pre_mpm requires a process-pool.
And, you
need to find some way to pass the scoreboard back to the child process
because you are about to start a new one.
What are you talking about, we killed Kenny :0/ This next generation we
have new offspring.
Re: cvs commit: httpd-2.0/server/mpm/winnt mpm_winnt.c
wrowe 02/03/19 22:14:19
Modified:server/mpm/winnt mpm_winnt.c
Log:
Fix a few listener-related lifetime issues [they are created in the
open logs phase, only once.]
-if (!set_listeners_noninheritable(pconf)) {
+if (!set_listeners_noninheritable(s-process-pool)) {
Ok guys, this really, really sucks.
Listeners must survive the server restart, to be closed down
in ap_open_listen after the new listener is created. Is noone
watching pool scope here?
I think once we all [me included] pay better attention to pool scope,
our Apache 2.0 will be the best server yet. But with the migration
to APR and tighter reigns on pool lifetimes, this is going to be
painful for a while.
The alternative is to allow one pconf to survive server restart until after
the -next- post-config hook is run, but that seems like overkill.
Bill
Re: cvs commit: httpd-2.0/server/mpm/winnt mpm_winnt.c mpm_winnt.h service.c
Bill, does this resolve the Win32 showstoppers? Specifically... * Address popular PRs * Win32 doesn't install as service correctly [9863, 9914, 9961] * Don't be stupid and cd to a blank directory when doing installs [PR 9993] I'll not be too keen on Win32 holding up a beta. Bill - Original Message - From: [EMAIL PROTECTED] To: [EMAIL PROTECTED] Sent: Wednesday, March 13, 2002 11:34 PM Subject: cvs commit: httpd-2.0/server/mpm/winnt mpm_winnt.c mpm_winnt.h service.c wrowe 02/03/13 20:34:03 Modified:server/mpm/winnt mpm_winnt.c mpm_winnt.h service.c Log: My [sixth?] major revamp of service.c. Traded an event for a mutex to the service_init completion, expanded timeouts, moved SERVICE_STOPPED message posting to the main thread since sometimes, in odd cirumstances, our SCM thread wasn't resumed prior to termination, and ripped the code for the stderr logs to use nt_eventlog.c instead. And generally tried to make the code just a little bit more grokable [as if such a thing is really possible.]
Re: cvs commit: httpd-2.0/server/mpm/winnt mpm_winnt.c mpm_winnt.h service.c
At 12:07 PM 3/14/2002, you wrote: Bill, does this resolve the Win32 showstoppers? Specifically... * Address popular PRs * Win32 doesn't install as service correctly [9863, 9914, 9961] * Don't be stupid and cd to a blank directory when doing installs [PR 9993] I'll not be too keen on Win32 holding up a beta. No, this doesn't fix those showstoppers. They actually don't belong in httpd-2.0/STATUS in the first place. They are part of the installer, not core, and belong in httpd-win32-msi/STATUS. And they can be fixed even after a 2.0.x is made beta-ready, since the tag on httpd-win32-msi is independent of httpd-2.0/apr/apr-util. Bill
Re: cvs commit: httpd-2.0/server/mpm/winnt mpm_winnt.c mpm_winnt.h service.c
c mpm_winnt.h service.c At 12:07 PM 3/14/2002, you wrote: Bill, does this resolve the Win32 showstoppers? Specifically... * Address popular PRs * Win32 doesn't install as service correctly [9863, 9914, 9961] * Don't be stupid and cd to a blank directory when doing installs [PR 9993] I'll not be too keen on Win32 holding up a beta. No, this doesn't fix those showstoppers. They actually don't belong in httpd-2.0/STATUS in the first place. They are part of the installer, not core, and belong in httpd-win32-msi/STATUS. And they can be fixed even after a 2.0.x is made beta-ready, since the tag on httpd-win32-msi is independent of httpd-2.0/apr/apr-util. That's what I thought. Cool! Bill
Re: cvs commit: httpd-2.0/server/mpm/winnt mpm_winnt.c
The important thing here is that the process yields to allow the child to initialize.
The
time of the sleep is really not important. The thing that needs to happen is deep in
the
OS, so I am not sure what we would mutex against. Yea, this does suck... it is a bug in
the OS AFAIK cause we shouldn;t have to jump through these hoops.
Bill
wrowe 02/02/08 11:37:03
Modified:server/mpm/winnt mpm_winnt.c
Log:
Drawing attention to the timing problem; push the wait up so we do about
nothing until WinNT initializes the app.
Revision ChangesPath
1.229 +10 -10httpd-2.0/server/mpm/winnt/mpm_winnt.c
Index: mpm_winnt.c
===
RCS file: /home/cvs/httpd-2.0/server/mpm/winnt/mpm_winnt.c,v
retrieving revision 1.228
retrieving revision 1.229
diff -u -r1.228 -r1.229
--- mpm_winnt.c 6 Feb 2002 21:09:26 - 1.228
+++ mpm_winnt.c 8 Feb 2002 19:37:02 - 1.229
@@ -1585,6 +1585,16 @@
NULL,
si, pi);
+/* Important:
+ * Give the child process a chance to run before dup'ing the sockets.
+ * We have already set the listening sockets noninheritable, but if
+ * WSADuplicateSocket runs before the child process initializes
+ * the listeners will be inherited anyway.
+ *
+ * XXX: This is badness; needs some mutex interlocking
+ */
+Sleep(1000);
+
/* Undo everything we created for the child only
*/
CloseHandle(pi.hThread);
@@ -1605,16 +1615,6 @@
ap_log_error(APLOG_MARK, APLOG_INFO, APR_SUCCESS, ap_server_conf,
Parent: Created child process %d, pi.dwProcessId);
-
-/* Important:
- * Give the child process a chance to run before dup'ing the sockets.
- * We have already set the listening sockets noninheritable, but if
- * WSADuplicateSocket runs before the child process initializes
- * the listeners will be inherited anyway.
- *
- * XXX: This is badness; needs some mutex interlocking
- */
-Sleep(1000);
if (send_handles_to_child(p, *child_exit_event, pi.hProcess, hPipeWrite)) {
CloseHandle(hPipeWrite);
RE: cvs commit: httpd-2.0/server/mpm/winnt mpm_winnt.c
Can we fix the service restart problem before we go making other changes to the MPM? I see over the last few days there have been dozens of changes to the code, none of which are directed at fixing the service restart problem. All this monkeying around with the code does is obscure finding what patch introduced the problem. Bleh... I disagree completely. The Windows MPM is fragile, because it is almost impossible to read. Bill has been cleaning it up so that multiple people can easily modify the code and actually understand what is happening. Knowing how much time Bill has spent reading through that code, and trying to solve the underlying problem, I know that he believes he must clean the code before he can fix the actual bug. Ryan
Re: cvs commit: httpd-2.0/server/mpm/winnt mpm_winnt.c
Can we fix the service restart problem before we go making other changes to the MPM? I see over the last few days there have been dozens of changes to the code, none of which are directed at fixing the service restart problem. All this monkeying around with the code does is obscure finding what patch introduced the problem. Bleh... I disagree completely. The Windows MPM is fragile, because it is almost impossible to read. Bill has been cleaning it up so that multiple people can easily modify the code and actually understand what is happening. Knowing how much time Bill has spent reading through that code, and trying to solve the underlying problem, I know that he believes he must clean the code before he can fix the actual bug. Ryan I suggest spending time to learn about code before you begin hacking it up. I've seen this exact same pattern repeat itself in Apache 2.0 multiple times. You and Bill have a grand vision of 'how things should be', and start making changes. When you are done, what once worked is broken and others are left to pick up the pieces and finish what you started. Both you guys have done some really good things, don't get me wrong, but I am frankly tired of picking up after you guys. When I stopped work on the windows MPM, everything worked. I leave it alone for several months, you guys come in and start changing things and you f*cked it up. A bit of testing after every change would have caught the bug when it was first introduced. It's not too much to ask that yuou finish what you start and understand what you are changing before you change it. You CANNOT argue you did this with the Windows MPM, because clearly you did not. Bill
RE: cvs commit: httpd-2.0/server/mpm/winnt mpm_winnt.c
I disagree completely. The Windows MPM is fragile, because it is almost impossible to read. Bill has been cleaning it up so that multiple people can easily modify the code and actually understand what is happening. Knowing how much time Bill has spent reading through that code, and trying to solve the underlying problem, I know that he believes he must clean the code before he can fix the actual bug. Ryan I suggest spending time to learn about code before you begin hacking it up. I've seen this exact same pattern repeat itself in Apache 2.0 multiple times. You and Bill have a grand vision of 'how things should be', and start making changes. When you are done, what once worked is broken and others are left to pick up the pieces and finish what you started. Both you guys have done some really good things, don't get me wrong, but I am frankly tired of picking up after you guys. When I stopped work on the windows MPM, everything worked. I leave it alone for several months, you guys come in and start changing things and you f*cked it up. A bit of testing after every change would have caught the bug when it was first introduced. It's not too much to ask that yuou finish what you start and understand what you are changing before you change it. You CANNOT argue you did this with the Windows MPM, because clearly you did not. BS I didn't test. I did test. I ran the server for three days after making my changes. In fact, Bill was harping on my to commit the damn patch because I was taking so long to get it in. The problem was that I didn't realize how fragile the damn service stuff was. Nor could I have, because there was no comment explaining what was happening. Add to that, the Windows MPM didn't behave like any other MPM. That means that as people were writing modules for Windows, they were adding hacks to their modules to make up for the way that the MPM did things. That is completely bogus. What I did was to move the Windows MPM back into line with the other MPMs with regard to the pre_mpm hook, so that it was very clearly stated that the pre_mpm hook is called once per Apache instance and the child_init hook is called once per child process. What Bill is doing is cleaning the code so that it looks like other Apache modules and can be followed. Does this mean that he is ripping the module apart and putting it back together? Yes, because that is what is required to understand the damn thing. As for the fact that the Windows MPM was working before we touched it and wasn't working afterwards, I don't believe it. I believe that the changes we made highlighted problems that weren't hitting due to luck before. Do I have proof? How about the fact that all I did was relocate code that was already in the server? I didn't add a single line of code, nor did I remove one. I just moved it from the pre_mpm phase to the child_init phase, which was not all that different at the end of the day. Ryan
