Re: Crash in scoreboard for 2.3.4 after restart (register_hooks)

2009-12-02 Thread Ruediger Pluem


On 12/02/2009 09:09 PM, Rainer Jung wrote:
> Platform: Solaris 8 (sic!)
> MPM: worker dynamically loaded
> APR etc: Bundled
> PCRE: 7.8
> 
> During testing of 2.3.4 I noticed crashes after restart.
> 
> I did a build with lots of modules, especially including mod_logio. The
> scoreboard uses in ap_increment_counts() the optional function
> ap_logio_get_last_bytes() from mod_logio if available.
> 
> In my case after restart the memory location of mod_logio and the
> address of the optional function changed, but the scoreboard still tries
> to call to the original address retrieved after the initial start.
> 
> I don't know about the full implementation of the optional functions,
> but it seems either
> 
> APR_REGISTER_OPTIONAL_FN(ap_logio_get_last_bytes);
> 
> in register_hooks in mod_logio needs to run after restarts too, or there
> is a problem resulting in an unwanted change of load order of the
> modules during restart. I did not edit the config files between start
> and restart.
> 
> The problem happens with normal restarts and graceful restarts.
> 
> Wild guess: it might have to do with dynamic MPM loading.

I am not so sure. I guess the problem is that we call

APR_RETRIEVE_OPTIONAL_FN(ap_logio_get_last_bytes);

only in ap_calc_scoreboard_size which is IMHO not called at restarts.
Does the following totally untested patch fix your issue?

Index: scoreboard.c
===
--- scoreboard.c(Revision 886294)
+++ scoreboard.c(Arbeitskopie)
@@ -284,6 +284,8 @@
 apr_status_t rv;
 #endif

+pfn_ap_logio_get_last_bytes = 
APR_RETRIEVE_OPTIONAL_FN(ap_logio_get_last_bytes);
+
 if (ap_scoreboard_image) {
 running_gen = ap_scoreboard_image->global->running_generation;
 ap_scoreboard_image->global->restart_time = apr_time_now();


Regards

Rüdiger



Re: Crash in scoreboard for 2.3.4 after restart (register_hooks)

2009-12-02 Thread Jeff Trawick
On Wed, Dec 2, 2009 at 3:33 PM, Ruediger Pluem  wrote:
>
>
> On 12/02/2009 09:09 PM, Rainer Jung wrote:
>> Platform: Solaris 8 (sic!)
>> MPM: worker dynamically loaded
>> APR etc: Bundled
>> PCRE: 7.8
>>
>> During testing of 2.3.4 I noticed crashes after restart.
>>
>> I did a build with lots of modules, especially including mod_logio. The
>> scoreboard uses in ap_increment_counts() the optional function
>> ap_logio_get_last_bytes() from mod_logio if available.
>>
>> In my case after restart the memory location of mod_logio and the
>> address of the optional function changed, but the scoreboard still tries
>> to call to the original address retrieved after the initial start.
>>
>> I don't know about the full implementation of the optional functions,
>> but it seems either
>>
>> APR_REGISTER_OPTIONAL_FN(ap_logio_get_last_bytes);
>>
>> in register_hooks in mod_logio needs to run after restarts too, or there
>> is a problem resulting in an unwanted change of load order of the
>> modules during restart. I did not edit the config files between start
>> and restart.
>>
>> The problem happens with normal restarts and graceful restarts.
>>
>> Wild guess: it might have to do with dynamic MPM loading.
>
> I am not so sure. I guess the problem is that we call
>
> APR_RETRIEVE_OPTIONAL_FN(ap_logio_get_last_bytes);
>
> only in ap_calc_scoreboard_size which is IMHO not called at restarts.

That was my guess.  Which reminds me of this old challenge for a module I wrote:

 * XXX
 * our daemon must be created in the pre-mpm hook so that the scoreboard
 * is available in the daemon process; the scoreboard isn't created until
 * the core module's pre-mpm hook
 * BUT: the pre-mpm hook is not called for graceful restart, so we can't
 *  start a new instance of the daemon then
 * BUT: request processing from the daemon MUST use the new configuration,
 *  so we'd be busted for graceful restart


Re: Crash in scoreboard for 2.3.4 after restart (register_hooks)

2009-12-02 Thread Rainer Jung

Hi Rüdiger,

On 02.12.2009 21:33, Ruediger Pluem wrote:



On 12/02/2009 09:09 PM, Rainer Jung wrote:

Platform: Solaris 8 (sic!)
MPM: worker dynamically loaded
APR etc: Bundled
PCRE: 7.8

During testing of 2.3.4 I noticed crashes after restart.

I did a build with lots of modules, especially including mod_logio. The
scoreboard uses in ap_increment_counts() the optional function
ap_logio_get_last_bytes() from mod_logio if available.

In my case after restart the memory location of mod_logio and the
address of the optional function changed, but the scoreboard still tries
to call to the original address retrieved after the initial start.

I don't know about the full implementation of the optional functions,
but it seems either

APR_REGISTER_OPTIONAL_FN(ap_logio_get_last_bytes);

in register_hooks in mod_logio needs to run after restarts too, or there
is a problem resulting in an unwanted change of load order of the
modules during restart. I did not edit the config files between start
and restart.

The problem happens with normal restarts and graceful restarts.

Wild guess: it might have to do with dynamic MPM loading.


I am not so sure. I guess the problem is that we call

APR_RETRIEVE_OPTIONAL_FN(ap_logio_get_last_bytes);

only in ap_calc_scoreboard_size which is IMHO not called at restarts.
Does the following totally untested patch fix your issue?

Index: scoreboard.c
===
--- scoreboard.c(Revision 886294)
+++ scoreboard.c(Arbeitskopie)
@@ -284,6 +284,8 @@
  apr_status_t rv;
  #endif

+pfn_ap_logio_get_last_bytes = 
APR_RETRIEVE_OPTIONAL_FN(ap_logio_get_last_bytes);
+
  if (ap_scoreboard_image) {
  running_gen = ap_scoreboard_image->global->running_generation;
  ap_scoreboard_image->global->restart_time = apr_time_now();




Yes, that fixes it, makes a lot of sense.

What's interesting is, that the function address changes during the 
first restart, but not any more when doing further restarts. So the 
initial start ends up with a different load order than all of the 
following restarts. Something to keep in mind at least in case it's new 
in 2.3 - and about 60 other places to check which use optional 
functions, whether the function pointers are correctly reinitialized.


Thanks!

Will you commit?

Rainer




Re: Crash in scoreboard for 2.3.4 after restart (register_hooks)

2009-12-02 Thread Ruediger Pluem


On 12/02/2009 09:53 PM, Rainer Jung wrote:

> 
> Yes, that fixes it, makes a lot of sense.
> 
> What's interesting is, that the function address changes during the
> first restart, but not any more when doing further restarts. So the
> initial start ends up with a different load order than all of the
> following restarts. Something to keep in mind at least in case it's new
> in 2.3 - and about 60 other places to check which use optional
> functions, whether the function pointers are correctly reinitialized.
> 
> Thanks!
> 
> Will you commit?

Done as r886324.

Regards

Rüdiger