[PATCH] Discussion of apache -k start -n apache2 problem...

[Bill Stoddard <bi...@wstoddard.com>]

This patch allows apache -k start -n apache2 to start the server.  This is certainly not
the correct fix but it should illustrate the problem for someone more familier with the
services code.  The problem (step-by-step):

1. Issue apache -k restart -n apache2
2. The SCM issues apache -k runservice (using values from the registry)
3. winnt_rewrite_args rewrites the args to

   apache -d c:/apache2/  (ie, apache -d server_root)

and saves the rewritten args in a global variable, mpm_new_argv. Everything is okay to
this point (I think :-). I.e.
mpm_new_argv[0] = c:/apache/bin/apache.exe
mpm_new_argv[1] = -d
mpm_new_argv[2] = server_root

Okay up to this point...

4. winnt_rewrite_args calls mpm_service_to_start, which among other things calls the Win32
call StartService() passing as one of its arguments, the string "apache2" (my patch passes
NULL instead)...

5. As a result of the STartService() call, Windows create a new thread which which calls
service_nt_main_fn (defined in service.c).  The code in service_nt_main_fn adds the
apache2 to the mpm_new_argv argument list, so the new argv looks like this:

apache -d c:/apache2/bin apache2

The trailing apache2 (which is the service name) is causing the (opt->ind < opt->argc)
check to fail and dumping us into usage() (from main.c)

    /* bad cmdline option?  then we die */
    if (rv != APR_EOF || opt->ind < opt->argc) {
        usage(process);
    }

I am thinking that the broken code is in service_nt_main_fn because it is not properly
updating the mpm_new_argv.

Another problem is that there is a race condition between the Win32 thread that gets
kicked off by the call to StartService() and the thread that is in mpm_rewrite_args.
Really nasty. A good Chianti and some grated parmesan and We'd have a meal :-)

Index: service.c
===================================================================
RCS file: /home/cvs/httpd-2.0/server/mpm/winnt/service.c,v
retrieving revision 1.51
diff -u -r1.51 service.c
--- service.c 17 May 2002 11:11:39 -0000 1.51
+++ service.c 3 Jun 2002 21:04:38 -0000
@@ -1059,12 +1061,11 @@

         argc += 1;
         start_argv = malloc(argc * sizeof(const char **));
-        start_argv[0] = mpm_service_name;
         if (argc > 1)
             memcpy(start_argv + 1, argv, (argc - 1) * sizeof(const char **));

         rv = APR_EINIT;
-        if (StartService(schService, argc, start_argv)
+        if (StartService(schService, 0, NULL)
             && signal_service_transition(schService, 0, /* test only */
                                          SERVICE_START_PENDING,
                                          SERVICE_RUNNING))

Re: [PATCH] Discussion of apache -k start -n apache2 problem...

[Juergen Heckel <JH...@t-online.de>]

Bill Stoddard wrote:
>
> This patch allows apache -k start -n apache2 to start the server.  This is certainly not
> the correct fix but it should illustrate the problem for someone more familier with the
> services code.  The problem (step-by-step):
>

Hi,
thank you.
This patch works for me:-)

Juergen

Re: [PATCH] Discussion of apache -k start -n apache2 problem...

["William A. Rowe, Jr." <wr...@rowe-clan.net>]

At 04:33 PM 6/3/2002, you wrote:
>This patch allows apache -k start -n apache2 to start the server.  This is 
>certainly not
>the correct fix but it should illustrate the problem for someone more 
>familier with the
>services code.  The problem (step-by-step):
>
>4. winnt_rewrite_args calls mpm_service_to_start, which among other things 
>calls the Win32
>call StartService() passing as one of its arguments, the string "apache2" 
>(my patch passes
>NULL instead)...

   Well that would be a bug, this should be -n Apache2 [two new args, not one]
so that the server can identify itself.

>5. As a result of the STartService() call, Windows create a new thread 
>which which calls
>service_nt_main_fn (defined in service.c).  The code in service_nt_main_fn 
>adds the
>apache2 to the mpm_new_argv argument list, so the new argv looks like this:
>
>apache -d c:/apache2/bin apache2

Well the missing -n prefix before apache2 would account for that.

>Another problem is that there is a race condition between the Win32 thread 
>that gets
>kicked off by the call to StartService() and the thread that is in 
>mpm_rewrite_args.
>Really nasty. A good Chianti and some grated parmesan and We'd have a meal :-)
>
>Index: service.c
>===================================================================
>RCS file: /home/cvs/httpd-2.0/server/mpm/winnt/service.c,v
>retrieving revision 1.51
>diff -u -r1.51 service.c
>--- service.c 17 May 2002 11:11:39 -0000 1.51
>+++ service.c 3 Jun 2002 21:04:38 -0000
>@@ -1059,12 +1061,11 @@
>
>          argc += 1;
>          start_argv = malloc(argc * sizeof(const char **));
>-        start_argv[0] = mpm_service_name;

-1 on the statics... need some

>          if (argc > 1)
>              memcpy(start_argv + 1, argv, (argc - 1) * sizeof(const char 
> **));
>
>          rv = APR_EINIT;
>-        if (StartService(schService, argc, start_argv)
>+        if (StartService(schService, 0, NULL)

You missed the point here.

If I ask for Apache -D SSL -k start ... I expect the SSL define to be USED
by the starting Apache.  You patch clobbers that.  Therefore it breaks the
ability to override ANY config option to Apache.

It sure looks like that stray service name sans -n is the culprit.  Should have
a fix in the hour.