httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Ohad Lutzky" <o...@lutzky.net>
Subject Re: [flood] Critical section for flood_report_relative_times.c
Date Sun, 21 Sep 2008 06:40:37 GMT
On Tue, Sep 16, 2008 at 9:45 PM, Justin Erenkrantz
<justin@erenkrantz.com> wrote:
> On Wed, Sep 3, 2008 at 2:28 AM, Ohad Lutzky <ohad@lutzky.net> wrote:
>> From flood_report_relative_times.c (SVN):
>>
>>    /* FIXME: this call may need to be in a critical section */
>> #if APR_HAS_THREADS
>>    apr_file_printf(local_stdout, "%s %ld %s\n", buf,
>> apr_os_thread_current(), req->uri);
>> #else
>>    apr_file_printf(local_stdout, "%s %d %s\n", buf, getpid(), req->uri);
>> #endif
>>
>> The comment is right - it does need to be in a critical section. For
>> the case of multiple processes, flock can be used. However, for the
>> case of a single process, this doesn't work, and a mutex would be in
>> order. What would be the correct way to go about organizing such a
>> per-process mutex? Note that apr_file_printf indirectly calls
>> apr_file_write, which is thread-safe - however, it call it multiple
>> times, which can (and does) lead to interlacing in output.
>
> Yup, I've seen that before, but I generally tend to ignore it.  =)
>
> I believe the best way would be to add a mutex in the config_t
> structure and lock that in the report function.  Rough patch below.
>
> What do you think?  -- justin
>
> Index: flood_farm.c
> ===================================================================
> --- flood_farm.c        (revision 695997)
> +++ flood_farm.c        (working copy)
> *SNIP* patch *SNIP*

The patch seems to work well, thanks! :)

-- 
Man is the only animal that laughs and weeps, for he is the only
animal that is struck with the difference between what things are and
what they ought to be.
 - William Hazlitt

Ohad Lutzky

Mime
View raw message