DO NOT REPLY [Bug 27241] New: - coding mistake relating to requests_this_child - it counts up and/or down depending on which source file you look at.

[bu...@apache.org]

DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG 
RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT
<http://nagoya.apache.org/bugzilla/show_bug.cgi?id=27241>.
ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND 
INSERTED IN THE BUG DATABASE.

http://nagoya.apache.org/bugzilla/show_bug.cgi?id=27241

coding mistake relating to requests_this_child - it counts up and/or down depending on which source file you look at.

           Summary: coding mistake relating to requests_this_child - it
                    counts up and/or down depending on which source file you
                    look at.
           Product: Apache httpd-2.0
           Version: 2.0.46
          Platform: All
        OS/Version: Other
            Status: NEW
          Severity: Normal
          Priority: Other
         Component: Core
        AssignedTo: bugs@httpd.apache.org
        ReportedBy: christopher@pobox.com


Hi,

Various parts of your source code think that "requests_this_child" counts 
upwards from 0 until it reaches "ap_max_requests_per_child" - but other parts 
of your code are expecting "requests_this_child" to have been *initialized* to 
ap_max_requests_per_child and they count downwards to <= 0.

Example1 (counts up):-

./server/mpm/prefork/prefork.c"  line 615

        if ((ap_max_requests_per_child > 0
             && requests_this_child++ >= ap_max_requests_per_child)) {  // 
cnd - hmm. sus.
            clean_child_exit(0);
        }

Example2 (counts down):-

./server/mpm/worker/worker.c" line 711

        if (requests_this_child <= 0) {
            check_infinite_requests();
        }

Example3 (discrepancies):-

[httpd-2.0.46]# grep -r 'requests_this_child--' .
./server/mpm/beos/beos.c:                requests_this_child--;
./server/mpm/experimental/leader/leader.c:                requests_this_child--
;
./server/mpm/experimental/perchild/perchild.c:            requests_this_child--
;
./server/mpm/experimental/threadpool/threadpool.c:        requests_this_child--
; /* FIXME: should be synchronized - aaron */
./server/mpm/worker/worker.c:        requests_this_child--; /* FIXME: should 
be synchronized - aaron */
./server/mpm/worker/worker.c.guardarea:        requests_this_child--; /* 
FIXME: should be synchronized - aaron */

[root@my httpd-2.0.46]# grep -r 'requests_this_child++' .  
./server/mpm/netware/mpm_netware.c:            && requests_this_child++ >= 
ap_max_requests_per_child)) {        // cnd - looks like a bug!
./server/mpm/mpmt_os2/mpmt_os2_child.c:            requests_this_child++;
./server/mpm/prefork/prefork.c:      && requests_this_child++ >= 
ap_max_requests_per_child)) {  // cnd - hmm. sus.
./server/mpm/prefork/prefork.c.graceful:             && requests_this_child++ 
>= ap_max_requests_per_child)) {
./server/mpm/prefork/prefork.c.xfsz:         && requests_this_child++ >= 
ap_max_requests_per_child)) {
./server/mpm/prefork/prefork.c.can0254:      && requests_this_child++ >= 
ap_max_requests_per_child)) {
./server/mpm/winnt/child.c:            requests_this_child++;


Example4 (various different ideas of how to initialize it):-
[httpd-2.0.46]# grep -r 'int requests_this_child' .  

./server/mpm/beos/beos.c:    int requests_this_child = 
ap_max_requests_per_thread;

./server/mpm/experimental/leader/leader.c:static int requests_this_child;
if (ap_max_requests_per_child) {
        requests_this_child = ap_max_requests_per_child;
    }
    else {
        /* coding a value of zero means infinity */
        requests_this_child = INT_MAX;
    }

./server/mpm/experimental/perchild/perchild.c:static int requests_this_child;
    requests_this_child = ap_max_requests_per_child;

./server/mpm/experimental/threadpool/threadpool.c:static int 
requests_this_child;
    requests_this_child = ap_max_requests_per_child;

./server/mpm/netware/mpm_netware.c:    int requests_this_child = 0;
./server/mpm/mpmt_os2/mpmt_os2_child.c:    int requests_this_child = 0;
./server/mpm/prefork/prefork.c:static int requests_this_child;
 requests_this_child = 0;
./server/mpm/prefork/prefork.c.graceful:static int requests_this_child;
 requests_this_child = 0;
./server/mpm/prefork/prefork.c.xfsz:static int requests_this_child;
 requests_this_child = 0;
./server/mpm/prefork/prefork.c.can0254:static int requests_this_child;
 requests_this_child = 0;
./server/mpm/winnt/child.c:    static int requests_this_child = 0;
./server/mpm/worker/worker.c:static int requests_this_child;
   requests_this_child = ap_max_requests_per_child;

./server/mpm/worker/worker.c.guardarea:static int requests_this_child;


I reccomend "requests_this_child" should mean "number of requests by this 
child" so it should count up, and never be initialized to 
ap_max_requests_per_child (at least, not without renaming it 
to "requests_remaining" or some such thing).


While on the subject - I find that when all my HTTP server processes 
reach "ap_max_requests_per_child" at the same time, it chokes my machine to 
death while they all restart at once.  It would be a good idea to add some 
small random amount at initialisation so that it is unlikley that all children 
will simultaneously die.  This is what I was doing when I noticed this bug.  
My "fix" [which was:  + (int) (42.0*rand()/(RAND_MAX+1.0)); ] solved my 
problem on several earlier apache releases, which is why I was adding it now.

---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@httpd.apache.org
For additional commands, e-mail: bugs-help@httpd.apache.org