Return-Path: 
 <new-httpd-return-5706-apmail-new-httpd-archive=apache.org@apache.org>
Delivered-To: apmail-new-httpd-archive@apache.org
Received: (qmail 90846 invoked by uid 500); 28 Jun 2000 01:21:57 -0000
Mailing-List: contact new-httpd-help@apache.org; run by ezmlm
Precedence: bulk
X-No-Archive: yes
Reply-To: new-httpd@apache.org
list-help: <mailto:new-httpd-help@apache.org>
list-unsubscribe: <mailto:new-httpd-unsubscribe@apache.org>
list-post: <mailto:new-httpd@apache.org>
Delivered-To: mailing list new-httpd@apache.org
Received: (qmail 90831 invoked from network); 28 Jun 2000 01:21:56 -0000
From: "William A. Rowe, Jr." <wrowe@lnd.com>
To: <new-httpd@apache.org>
Subject: Please Review: [1.3.13 patch] Win32 Hold Console Open,
 Log stderr to Event Log, and fix Child stdin Pipe
Date: Tue, 27 Jun 2000 20:21:23 -0500
Message-ID: <000401bfe09f$41405420$345985d0@corecomm.net>
MIME-Version: 1.0
Content-Type: multipart/mixed;
	boundary="----=_NextPart_000_0005_01BFE075.586A4C20"
X-Priority: 3 (Normal)
X-MSMail-Priority: Normal
X-Mailer: Microsoft Outlook 8.5, Build 4.71.2173.0
X-MimeOLE: Produced By Microsoft MimeOLE V5.00.2919.6600
Importance: Normal
X-Spam-Rating: locus.apache.org 1.6.2 0/1000/N

This is a multi-part message in MIME format.

------=_NextPart_000_0005_01BFE075.586A4C20
Content-Type: text/plain;
	charset="iso-8859-1"
Content-Transfer-Encoding: 7bit

So,

  This is the patch I will apply at noon tommorow if I have
sufficient votes.  I'm not taking it on myself to decide
"we should do this", I'm simply going to offer the patch
since they are 'features', not fixes.  I know a -few- of you
would rather see my attention directed at 2.0 - your review
of this patch will help make that happen.

  First it fixes what I -BELIEVE- is a bug in http_main
child spawning that keeps the pipe open when it shouldn't be.
Since the parent end of the pipe is inheritable, it doesn't
clamp down when the parent dies an unexpected death.

  The patch keeps the Win32 user's console open for up to
30 seconds so they can read the silly error message, or
they can press <ESC> or double click on the window.  It does
this with a MACRO WRAPPER over exit() - so careful inspection
is well advised :)

  The patch reports service startup errors for WinNT in the
Application event log.  The code to reroute stderr is 
bare-butt ugly, but I could think of no other way to get
out of the buffered nightmare we are in (this is also why
things never hit the logs, of course.  fflush() won't even
do what it says it does.)  If someone can get it running
and make that fragment a little more ellegant, be my guest.

  The nice thing, we cheat.  No message file in 1.3.13 - we
just steal the netmsg.dll standard %1 %2 %3 %4 %5 %6 %7 %8 %9
message for what we need.  2.0 will have a message file, I
expect, to break out all the log conditions if the WinNT user
chooses syslog for output.

  That is all.  Time to port forward to 2.0, after a good nap.

Bill


  

------=_NextPart_000_0005_01BFE075.586A4C20
Content-Type: text/plain;
	name="win32info.diffs"
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment;
	filename="win32info.diffs"

? os/win32/t
? os/win32/Apache.aps
? os/win32/Apache.mc
? os/win32/ApacheLog.mc
? os/win32/ApacheLog.h
? os/win32/ApacheLog.rc
? os/win32/ApacheLog.bin
Index: Apache.dsp
===================================================================
RCS file: /home/cvs/apache-1.3/src/Apache.dsp,v
retrieving revision 1.11
diff -u -r1.11 Apache.dsp
--- Apache.dsp	2000/06/23 00:17:51	1.11
+++ Apache.dsp	2000/06/28 01:09:05
@@ -65,8 +65,8 @@
 # PROP Intermediate_Dir "ApacheD"
 # PROP Ignore_Export_Lib 0
 # PROP Target_Dir ""
-# ADD BASE CPP /nologo /W3 /Gm /GX /ZI /Od /D "WIN32" /D "_DEBUG" /D "_CONSOLE" /c
-# ADD CPP /nologo /MDd /W3 /Gm /GX /ZI /Od /D "WIN32" /D "_DEBUG" /D "_CONSOLE" /FD /c
+# ADD BASE CPP /nologo /W3 /GX /Od /D "WIN32" /D "_DEBUG" /D "_CONSOLE" /ZI /c
+# ADD CPP /nologo /MDd /W3 /GX /Od /D "WIN32" /D "_DEBUG" /D "_CONSOLE" /FD /ZI /c
 # ADD BASE RSC /l 0x809 /d "_DEBUG"
 # ADD RSC /l 0x809 /d "_DEBUG"
 BSC32=bscmake.exe
@@ -100,6 +100,10 @@
 # Begin Source File
 
 SOURCE=.\os\win32\apache.ico
+# End Source File
+# Begin Source File
+
+SOURCE=.\os\win32\Apache.mc
 # End Source File
 # Begin Source File
 
Index: main/http_main.c
===================================================================
RCS file: /home/cvs/apache-1.3/src/main/http_main.c,v
retrieving revision 1.503
diff -u -r1.503 http_main.c
--- main/http_main.c	2000/06/16 18:31:04	1.503
+++ main/http_main.c	2000/06/28 01:09:23
@@ -5677,7 +5677,7 @@
         
         ap_destroy_pool(pchild);
         cleanup_scoreboard();
-        exit(0);
+        exit(1);
     }
     
     set_signals();
@@ -5879,7 +5879,7 @@
 
 	ap_destroy_pool(pchild);
 	cleanup_scoreboard();
-	exit(0);
+	exit(1);
     }
     if (rv == WAIT_OBJECT_0 + 1) {
 	/* exit event signalled - exit now */
@@ -5904,7 +5904,7 @@
 
 	ap_destroy_pool(pchild);
 	cleanup_scoreboard();
-	exit(0);
+	exit(1);
     }
     set_signals();
 
@@ -6118,6 +6118,8 @@
     DWORD BytesWritten;
     HANDLE hPipeRead = NULL;
     HANDLE hPipeWrite = NULL;
+    HANDLE hPipeWriteDup;
+    HANDLE hCurrentProcess;
     SECURITY_ATTRIBUTES sa = {0};  
 
     sa.nLength = sizeof(sa);
@@ -6166,6 +6168,14 @@
         return -1;
     }
 
+    hCurrentProcess = GetCurrentProcess();
+    if (DuplicateHandle(hCurrentProcess, hPipeWrite, hCurrentProcess,
+                        &hPipeWriteDup, 0, FALSE, DUPLICATE_SAME_ACCESS))
+    {
+        CloseHandle(hPipeWrite);
+        hPipeWrite = hPipeWriteDup;
+    }
+
     /* Give the read in of the pipe (hPipeRead) to the child as stdin. The 
      * parent will write the socket data to the child on this pipe.
      */
@@ -6626,6 +6636,9 @@
             clean_parent_exit(0);
         }
     }
+
+    /* This behavior is voided by setting real_exit_code to 0 */
+    atexit(hold_console_open_on_error);
 #endif
 
 #ifdef NETWARE
@@ -6679,6 +6692,8 @@
 	    break;
 #ifdef WIN32
 	case 'Z':
+            /* Prevent holding open the (nonexistant) console */
+            real_exit_code = 0;
 	    exit_event = open_event(optarg);
 	    APD2("child: opened process event %s", optarg);
 	    cp = strchr(optarg, '_');
@@ -6735,20 +6750,36 @@
 	    ap_set_version();
 	    printf("Server version: %s\n", ap_get_server_version());
 	    printf("Server built:   %s\n", ap_get_server_built());
+#ifdef NETWARE
             clean_parent_exit(0);
+#else
+            clean_parent_exit(1);
+#endif
 
-	case 'V':
+        case 'V':
 	    ap_set_version();
 	    show_compile_settings();
+#ifdef NETWARE
             clean_parent_exit(0);
+#else
+            clean_parent_exit(1);
+#endif
 
 	case 'l':
 	    ap_show_modules();
+#ifdef NETWARE
             clean_parent_exit(0);
+#else
+            clean_parent_exit(1);
+#endif
 
 	case 'L':
 	    ap_show_directives();
+#ifdef NETWARE
             clean_parent_exit(0);
+#else
+            clean_parent_exit(1);
+#endif
 
 	case 'X':
 	    ++one_process;	/* Weird debugging mode. */
@@ -6785,14 +6816,14 @@
         if (install > 0) {
             ap_log_error(APLOG_MARK, APLOG_NOERRNO|APLOG_ERR, NULL,
                          "Service \"%s\" is already installed!", service_name);
-            clean_parent_exit(0);
+            clean_parent_exit(1);
         }
     }
     else if (service_name && (install <= 0))
     {
         ap_log_error(APLOG_MARK, APLOG_NOERRNO|APLOG_ERR, NULL,
                      "Service \"%s\" is not installed!", service_name);
-        clean_parent_exit(0);
+        clean_parent_exit(1);
     }
 #endif
 
@@ -6851,16 +6882,20 @@
 
     if (service_name && !conf_specified) {
         printf("Unknown service: %s\n", service_name);
-        clean_parent_exit(0);
+        clean_parent_exit(1);
     }
 
     /* All NT signals, and all but the 9x start signal are handled entirely.
      * Die if we failed, are on NT, or are not "start"ing the service
      */
     if (service_name && signal_to_send) {
-        if (!send_signal_to_service(service_name, signal_to_send)
-                || isWindowsNT() || strcasecmp(signal_to_send, "start"))
+        if (send_signal_to_service(service_name, signal_to_send))
             clean_parent_exit(0);
+        if (isWindowsNT() || strcasecmp(signal_to_send, "start"))
+            clean_parent_exit(1);
+        /* Still here?  Then we are hanging around to detach the console 
+         * and use this process as the Windows 9x service.
+         */
     }
 #else /* ndef WIN32 */
     server_conf = ap_read_config(pconf, ptrans, ap_server_confname);
Index: os/win32/service.c
===================================================================
RCS file: /home/cvs/apache-1.3/src/os/win32/service.c,v
retrieving revision 1.23
diff -u -r1.23 service.c
--- os/win32/service.c	2000/06/16 18:31:13	1.23
+++ os/win32/service.c	2000/06/28 01:09:28
@@ -41,6 +41,71 @@
 static int ap_stop_service(SC_HANDLE);
 static int ap_restart_service(SC_HANDLE);
 
+/* exit() for Win32 is macro mapped (horrible, we agree) that allows us 
+ * to catch the non-zero conditions and inform the console process that
+ * the application died, and hang on to the console a bit longer.
+ *
+ * The macro only maps for http_main.c and other sources that include
+ * the service.h header, so we best assume it's an error to exit from
+ * _any_ other module.
+ *
+ * If real_exit_code is reset to 0, it will not be set or trigger this
+ * behavior on exit.  All service and child processes are expected to
+ * reset this flag to zero to avoid undesireable side effects.
+ */
+int real_exit_code = 1;
+
+void hold_console_open_on_error(void)
+{
+    HANDLE hConIn;
+    HANDLE hConErr;
+    DWORD result;
+    time_t start;
+    time_t remains;
+    char *msg = "Note the errors or messages above, "
+                "and press the <ESC> key to exit.  ";
+    CONSOLE_SCREEN_BUFFER_INFO coninfo;
+    INPUT_RECORD in;
+    char count[16];
+    
+    if (!real_exit_code)
+        return;
+    hConIn = GetStdHandle(STD_INPUT_HANDLE);
+    hConErr = GetStdHandle(STD_ERROR_HANDLE);
+    if ((hConIn == INVALID_HANDLE_VALUE) || (hConErr == INVALID_HANDLE_VALUE))
+        return;
+    if (!WriteConsole(hConErr, msg, strlen(msg), &result, NULL) || !result)
+        return;
+    if (!GetConsoleScreenBufferInfo(hConErr, &coninfo))
+        return;
+    if (!SetConsoleMode(hConIn, ENABLE_MOUSE_INPUT | 0x80))
+        return;
+        
+    start = time(NULL);
+    do
+    {
+        while (PeekConsoleInput(hConIn, &in, 1, &result) && result)
+        {
+            if (!ReadConsoleInput(hConIn, &in, 1, &result) || !result)
+                return;
+            if ((in.EventType == KEY_EVENT) && in.Event.KeyEvent.bKeyDown 
+                    && (in.Event.KeyEvent.uChar.AsciiChar == 27))
+                return;
+            if (in.EventType == MOUSE_EVENT 
+                    && (in.Event.MouseEvent.dwEventFlags == DOUBLE_CLICK))
+                return;
+        }
+        remains = ((start + 30) - time(NULL)); 
+        sprintf (count, "%d...", remains);
+        if (!SetConsoleCursorPosition(hConErr, coninfo.dwCursorPosition))
+            return;
+        if (!WriteConsole(hConErr, count, strlen(count), &result, NULL) 
+                || !result)
+            return;
+    }
+    while ((remains > 0) && WaitForSingleObject(hConIn, 1000) != WAIT_FAILED);
+}
+
 int service_main(int (*main_fn)(int, char **), int argc, char **argv )
 {
     SERVICE_TABLE_ENTRY dispatchTable[] =
@@ -49,6 +114,9 @@
         { NULL, NULL }
     };
 
+    /* Prevent holding open the (nonexistant) console */
+    real_exit_code = 0;
+
     globdat.main_fn = main_fn;
     globdat.stop_event = create_event(0, 0, "apache-signal");
     globdat.connected = 1;
@@ -171,7 +239,7 @@
     /* Windows 95/98 */
     char *service_name;
     HANDLE thread;
-    
+
     /* Remove spaces from display name to create service name */
     service_name = strdup(display_name);
     ap_remove_spaces(service_name, display_name);
@@ -191,12 +259,16 @@
     if (!RegisterServiceProcess((DWORD)NULL, 1))
         return -1;
 
+    /* Prevent holding open the (nonexistant) console */
+    real_exit_code = 0;
+
     /* Hide the console */
     FreeConsole();
 
     thread = CreateThread(NULL, 0, WatchWindow, (LPVOID) service_name, 0, 
 			  &monitor_thread_id);
-    CloseHandle(thread);
+    if (thread)
+        CloseHandle(thread);
 
     atexit(stop_service_monitor);
 
@@ -215,11 +287,87 @@
     chdir(buf);
 }
 
+long __stdcall service_stderr_thread(LPVOID hPipe)
+{
+    HANDLE hPipeRead = (HANDLE) hPipe;
+    HANDLE hEventSource;
+    char errbuf[256];
+    char *errmsg = errbuf;
+    char *errarg[9];
+    DWORD errlen = 0;
+    DWORD errres;
+    HKEY hk;
+    
+    errarg[0] = "The Apache service named";
+    errarg[1] = ap_server_argv0;
+    errarg[2] = "reported the following error:\r\n>>>";
+    errarg[3] = errmsg;
+    errarg[4] = "<<<\r\n before the error.log file could be opened.\r\n";
+    errarg[5] = "More information may be available in the error.log file.";
+    errarg[6] = NULL;
+    errarg[7] = NULL;
+    errarg[8] = NULL;
+    
+    /* What are we going to do in here, bail on the user?  not. */
+    if (!RegCreateKey(HKEY_LOCAL_MACHINE, "SYSTEM\\CurrentControlSet\\Services"
+                      "\\EventLog\\Application\\Apache Service", &hk)) 
+    {
+        /* The stock message file */
+        char *netmsgkey = "%SystemRoot%\\System32\\netmsg.dll";
+        DWORD dwData = EVENTLOG_ERROR_TYPE | EVENTLOG_WARNING_TYPE | 
+                       EVENTLOG_INFORMATION_TYPE; 
+ 
+        RegSetValueEx(hk, "EventMessageFile", 0, REG_EXPAND_SZ,
+                          (LPBYTE) netmsgkey, strlen(netmsgkey) + 1);
+        
+        RegSetValueEx(hk, "TypesSupported", 0, REG_DWORD,
+                          (LPBYTE) &dwData, sizeof(dwData));
+        RegCloseKey(hk);
+    }
+
+    hEventSource = RegisterEventSource(NULL, "Apache Service");
+
+    while (ReadFile(hPipeRead, errmsg, 1, &errres, NULL) && (errres == 1))
+    {
+        if ((errmsg > errbuf) || !isspace(*errmsg))
+        {
+            ++errlen;
+            ++errmsg;
+            if ((*(errmsg - 1) == '\n') || (errlen == sizeof(errbuf) - 1))
+            {
+                while (errlen && isspace(errbuf[errlen - 1]))
+                    --errlen;
+                errbuf[errlen] = '\0';
+
+                /* Generic message: '%1 %2 %3 %4 %5 %6 %7 %8 %9'
+                 * The event code in netmsg.dll is 3299
+                 */
+                ReportEvent(hEventSource, EVENTLOG_ERROR_TYPE, 0, 
+                            3299, NULL, 9, 0, errarg, NULL);
+                errmsg = errbuf;
+                errlen = 0;
+            }
+        }
+    }
+
+    CloseHandle(hPipeRead);
+    return 0;
+}
+
 void __stdcall service_main_fn(DWORD argc, LPTSTR *argv)
 {
+    HANDLE hCurrentProcess;
+    HANDLE hPipeRead = NULL;
+    HANDLE hPipeWrite = NULL;
+    HANDLE hPipeReadDup;
+    HANDLE thread;
+    DWORD  threadid;
+    SECURITY_ATTRIBUTES sa = {0};  
+    
     ap_server_argv0 = globdat.name = argv[0];
-
-    if(!(globdat.hServiceStatus = RegisterServiceCtrlHandler( globdat.name, service_ctrl)))
+    
+    if(!(globdat.hServiceStatus = RegisterServiceCtrlHandler(globdat.name, 
+                                                             service_ctrl)))
     {
         ap_log_error(APLOG_MARK, APLOG_ERR|APLOG_WIN32ERROR, NULL,
         "Failure registering service handler");
@@ -231,11 +379,54 @@
         NO_ERROR,              // exit code
         3000);                 // wait hint
 
+    /* Create a pipe to send stderr messages to the system error log */
+    hCurrentProcess = GetCurrentProcess();
+    if (CreatePipe(&hPipeRead, &hPipeWrite, &sa, 0)) 
+    {
+        if (DuplicateHandle(hCurrentProcess, hPipeRead, hCurrentProcess,
+                            &hPipeReadDup, 0, FALSE, DUPLICATE_SAME_ACCESS))
+        {
+            CloseHandle(hPipeRead);
+            hPipeRead = hPipeReadDup;
+            thread = CreateThread(NULL, 0, service_stderr_thread, 
+                                  (LPVOID) hPipeRead, 0, &threadid);
+            if (thread)
+            {
+                int fh;
+                FILE *fl;
+                CloseHandle(thread);
+            	fflush(stderr);
+		SetStdHandle(STD_ERROR_HANDLE, hPipeWrite);
+				
+                fh = _open_osfhandle((long) STD_ERROR_HANDLE, 
+                                     _O_WRONLY | _O_BINARY);
+                dup2(fh, STDERR_FILENO);
+                fl = _fdopen(STDERR_FILENO, "wcb");
+                memcpy(stderr, fl, sizeof(FILE));
+            }
+            else
+            {
+                CloseHandle(hPipeRead);
+                CloseHandle(hPipeWrite);
+                hPipeWrite = NULL;
+            }            
+        }
+        else
+        {
+            CloseHandle(hPipeRead);
+            CloseHandle(hPipeWrite);
+            hPipeWrite = NULL;
+        }            
+    }
+
     service_cd();
     if( service_init() ) 
         /* Arguments are ok except for \! */
         globdat.exit_status = (*globdat.main_fn)( argc, argv );
-    
+
+    if (hPipeWrite)
+        CloseHandle(hPipeWrite);
+
     ReportStatusToSCMgr(SERVICE_STOPPED, NO_ERROR, 0);
 
     return;
@@ -722,7 +913,8 @@
                  * will simply fall through and *THIS* process will fade into an
                  * invisible 'service' process, detaching from the user's console.
                  * We need to change the restart signal to "start", however,
-                 * if the service was not -yet- running.
+                 * if the service was not -yet- running, and we do return FALSE
+                 * to assure main() that we haven't done anything yet.
                  */
                 if (action == restart) 
                 {
@@ -731,7 +923,6 @@
                     else
                         ap_start_restart(1);
                 }
-                success = TRUE;
             }
         }
 
@@ -811,6 +1002,7 @@
     {
         case CTRL_C_EVENT:
         case CTRL_BREAK_EVENT:
+            real_exit_code = 0;
             fprintf(stderr, "Apache server interrupted...\n");
             /* for Interrupt signals, shut down the server.
              * Tell the system we have dealt with the signal
@@ -827,6 +1019,7 @@
              * after a reasonable time to tell the system
              * that we have already tried to shut down.
              */
+            real_exit_code = 0;
             fprintf(stderr, "Apache server shutdown initiated...\n");
             ap_start_shutdown();
             Sleep(30000);
@@ -884,7 +1077,8 @@
         HANDLE thread;
         thread = CreateThread(NULL, 0, WatchWindow, NULL, 0, 
                               &monitor_thread_id);
-        CloseHandle(thread);
+        if (thread)
+            CloseHandle(thread);
     }
 
     atexit(stop_console_monitor);
Index: os/win32/service.h
===================================================================
RCS file: /home/cvs/apache-1.3/src/os/win32/service.h,v
retrieving revision 1.8
diff -u -r1.8 service.h
--- os/win32/service.h	2000/06/12 00:41:23	1.8
+++ os/win32/service.h	2000/06/28 01:09:28
@@ -3,6 +3,19 @@
 #define SERVICE_H
 
 #ifdef WIN32
+
+/* BIG RED WARNING: exit() is mapped to allow us to capture the exit
+ * status.  This header must only be included from modules linked into
+ * the ApacheCore.dll - since it's a horrible behavior to exit() from
+ * any module outside the main() block, and we -will- assume it's a
+ * fatal error.  No dynamically linked module will ever be able to find
+ * the real_exit_code, and _will_ GP fault if it tries this macro.
+ */
+
+#define exit(status) ((exit)(real_exit_code ? (real_exit_code = (status)) : (status)))
+extern int real_exit_code;
+void hold_console_open_on_error(void);
+
 int service_main(int (*main_fn)(int, char **), int argc, char **argv);
 int service95_main(int (*main_fn)(int, char **), int argc, char **argv,
 		   char *display_name);

------=_NextPart_000_0005_01BFE075.586A4C20--