On Wed, Apr 11, 2012 at 12:08 AM, William A. Rowe Jr. <[email protected]> wrote: > On 4/10/2012 8:27 PM, Jeff Trawick wrote: >> On Tue, Apr 10, 2012 at 11:55 AM, William A. Rowe Jr. >> <[email protected]> wrote: >>> On 4/10/2012 10:31 AM, Jeff Trawick wrote: >>>> On Tue, Apr 10, 2012 at 12:05 AM, <[email protected]> wrote: >>>>> >>>>> + /* Cleanup the Job object if present */ >>>>> + conf = ap_get_module_config(((server_rec*)server)->module_config, >>>>> + &fcgid_module); >>>>> + >>>>> + if (conf != NULL && conf->hJobObjectForAutoCleanup != NULL) { >>>>> + CloseHandle(conf->hJobObjectForAutoCleanup); >>>>> + } >>>>> + >>>> >>>> Isn't it more idiomatic to register a cleanup for the job object >>>> rather than explicitly checking for whether or not it exists in >>>> different code? >>> >>> +1 >> >> I haven't touched that. After possibly wasting time hacking the error >> reporting/handling for when the job object is created, I wonder if >> this is even the right place to create the job object and potentially >> register a cleanup. Why not in a post-config hook? Also, is this >> really needed in parent AND child? > > The windows logic needs a lot more thought in relation to the parent and > child, where this pool of fcgid workers is created, how they are released. > > But as job objects, they will be gone as the parent dies, so I believe > the whole theory is fundamentally sound. Cosmetics like this do deserve > deeper consideration, but I think it's ready for release as is.
Agreed that it doesn't keep it from working and isn't going to hurt anyone... I'm done with "tweaking" this feature.
