Bug 28063 - Missing parameter in ap_log_error and lack of NULL checking in child.c:1051
Summary: Missing parameter in ap_log_error and lack of NULL checking in child.c:1051
Status: CLOSED FIXED
Alias: None
Product: Apache httpd-2
Classification: Unclassified
Component: Core (show other bugs)
Version: 2.0.49
Hardware: PC Windows XP
: P3 major (vote)
Target Milestone: ---
Assignee: Apache HTTPD Bugs Mailing List
URL:
Keywords: PatchAvailable
Depends on:
Blocks:
 
Reported: 2004-03-30 19:24 UTC by Eider Oliveira
Modified: 2004-11-16 19:05 UTC (History)
0 users


Attachments
Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Eider Oliveira 2004-03-30 19:24:50 UTC 
/* Kill remaining threads off the hard way */
    if (threads_created) {
        ap_log_error(APLOG_MARK,APLOG_NOTICE, APR_SUCCESS, ap_server_conf, 
                     "Child %d: Terminating %d threads that failed to exit.",
my_pid);
should be ==>>>      "Child %d: Terminating %d threads that failed to exit.",
my_pid, threads_created);


    }
    for (i = 0; i < threads_created; i++) {
        int *score_idx;
        TerminateThread(child_handles[i], 1);
        CloseHandle(child_handles[i]);
        /* Reset the scoreboard entry for the thread we just whacked */
        score_idx = apr_hash_get(ht, &child_handles[i], sizeof(HANDLE));
should insert ===> if (score_idx != NULL)
        ap_update_child_status_from_indexes(0, *score_idx, SERVER_DEAD, NULL); 
      
    }
Comment 1 Jeff Trawick 2004-03-30 20:18:12 UTC 
2.0.49 code has same issues
Comment 2 Jeff Trawick 2004-04-02 01:48:21 UTC 
The first part of the patch is clearly needed.

Can you verify that you had a crash that required the second change?  No problem
here with a bit of defensive programming in case the code above changes, but
since threads_created and hash table entries are created in lock step, I don't
see how there could be no hash entry.  Curious!
Comment 3 Eider Oliveira 2004-04-05 12:27:11 UTC 
I don't know why, but it crashed for me in this line. When I was looking into
the code, I've found the first wrong line and subimitted both of them.
I think it is related with pool cleanups.
Comment 4 Jeff Trawick 2004-08-14 10:59:55 UTC 
Nobody chimed in with an explanation for exactly why the "if (score_idx !=
NULL)" is needed, so I would prefer not to commit that.  However, that's no
reason to hold up the other fix, which should have been committed long ago.

Please re-open the PR if you find out why the extra check was needed.

Thanks for your fix!