Stas Bekman wrote:
>Steve Hay wrote:
>[...]
>
>
>>As you can see, the fname member is (presumably correctly) set to the
>>error_log path, but the mutex member is NULL.
>>
>>I may be wrong here, but it looks like the apr_file_t *logfile in
>>modperl_common_log.c is set by modperl_trace_logfile_set(), which is
>>called from modperl_hook_post_config(). The value being passed to
>>modperl_trace_logfile_set() is an apr_file_dup() of s->error_log:
>>
>> apr_file_t *dup;
>> MP_RUN_CROAK(apr_file_dup(&dup, s->error_log, pconf),
>> "mod_perl core post_config");
>> modperl_trace_logfile_set(dup);
>>
>>I walked through apr_file_dup() and watched what happened. s->error_log
>>has a (valid) mutex member, but the dup of it does not. Inside
>>apr_file_dup(), all the members of **new_file are NULL'ed by the
>>apr_pcalloc() call, and only some are then set to their correct values.
>>mutex is not one of them, so it just gets left NULL.
>>
>>
>
>Perfect, so you've uncovered a yet another bug in apr, on unix it does
>create the mutex, see: apr/file_io/unix/filedup.c: _file_dup
>
>So fix apr/file_io/win32/filedup.c: apr_file_dup to create the mutex,
>looking at the function above and apr_file_setaside in
>apr/file_io/win32/filedup.c, which does:
>
> if (old_file->mutex) {
> apr_thread_mutex_create(&((*new_file)->mutex),
> APR_THREAD_MUTEX_DEFAULT, p);
> apr_thread_mutex_destroy(old_file->mutex);
> }
>
>
OK, the attached patch (against httpd-2.0.50) gets things (mostly)
working for me.
I dropped the check for (*new_file)->buffered that the unix case has
because the win32 case always sets it to FALSE so the new if { ... }
block would not have been entered. (I considered copying ->buffered
from the old_file like unix does, but it was FALSE on the old_file too.)
I also dropped the !(*new_file)->mutex test that the unix case has
because on win32 we've always just done a apr_pcalloc() on the
*new_file, so we know ->mutex is NULL.
There's also a apr_file_dup2() in the win32 file. I don't know if that
needs fixing up similarly.
Should I send this patch somewhere else for consideration?
I tried starting apache.exe with each of the MOD_PERL_TRACE flags in
turn. Previously d, e, f, g, h, i, o and r all caused a crash. Now
they all work OK except for g (globals) which still causes a crash:
NTDLL! 77f69ecd()
NTDLL! 77f5b2e6()
apr_file_write(apr_file_t * 0x009e0268, const void * 0x009ed6b8,
unsigned int * 0x0006de94) line 274
apr_file_puts(const char * 0x009ed6b8, apr_file_t * 0x009e0268) line 399
apr_file_printf(apr_file_t * 0x009e0268, const char * 0x100363ec
`string') line 475 + 13 bytes
modperl_trace(const char * 0x100331b4 `string', const char * 0x100331cc
`string') line 51 + 22 bytes
modperl_global_cleanup(void * 0x100373bc MP_global_server_rec) line 79 +
34 bytes
run_cleanups(cleanup_t * * 0x0029acf8) line 1951 + 13 bytes
apr_pool_clear(apr_pool_t * 0x0029ace8) line 693 + 12 bytes
main(int 9, const char * const * 0x00292918) line 576
mainCRTStartup() line 338 + 17 bytes
KERNEL32! 77e814c7()
Don't know what the top two frames there are. They both show up as
"Disassembly" in the debugger :( The apr_file_t * in apr_file_write()
does have a mutex, though.
- Steve
------------------------------------------------
Radan Computational Ltd.
The information contained in this message and any files transmitted with it are
confidential and intended for the addressee(s) only. If you have received this
message in error or there are any problems, please notify the sender immediately. The
unauthorized use, disclosure, copying or alteration of this message is strictly
forbidden. Note that any views or opinions presented in this email are solely those
of the author and do not necessarily represent those of Radan Computational Ltd. The
recipient(s) of this message should check it and any attached files for viruses: Radan
Computational will accept no liability for any damage caused by any virus transmitted
by this email.
--- srclib/apr/file_io/win32/filedup.c.orig 2004-02-13 00:33:44.000000000 +0000
+++ srclib/apr/file_io/win32/filedup.c 2004-09-16 15:21:06.417830300 +0100
@@ -44,6 +44,13 @@
(*new_file)->buffered = FALSE;
(*new_file)->ungetchar = old_file->ungetchar;
+#if APR_HAS_THREADS
+ if (old_file->mutex) {
+ apr_thread_mutex_create(&((*new_file)->mutex),
+ APR_THREAD_MUTEX_DEFAULT, p);
+ }
+#endif
+
apr_pool_cleanup_register((*new_file)->pool, (void *)(*new_file), file_cleanup,
apr_pool_cleanup_null);
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
