236228 - NSPR LOG Buffering isn't WinDebug safe

Status: RESOLVED FIXED

(NSPR :: NSPR, defect)

Description

[timeless]

Steps:
build nspr
build nspr tests

set NSPR_LOG_FILE=WinDebug
set NSPR_LOG_MODULES=all:5
run logger.exe

there are two possible crashes. this stack is for the second:

crash:
 	ntdll.dll!RtlEnterCriticalSection()  + 0xb	
>	nspr4.dll!PR_LogFlush()  Line 487 + 0xe	C
 	logger.exe!main(int argc=0x00000001, const char * * argv=0x00342c38)  Line 153	C
 	logger.exe!mainCRTStartup()  Line 400 + 0xe	C
 	kernel32.dll!GetCurrentDirectoryW()  + 0x44

Comment 1

[timeless]

Attachment: possible patch (skips buffering for windebug)

Comment 2

[Wan-Teh Chang]

Is it possible to fix this bug while preserving
buffering for WinDebug?

Comment 3

[timeless]

Attachment: alternative path (tries to buffer on windows)

preserve? .. heh, that sounds as if buffering ever worked for windebug. it
didn't, it just crashed. Further, fputs() doesn't do anything with the nb param
to _PUT_LOG. So to be blunt, buffering and _PUT_LOG don't work for
_PR_USE_STDIO_FOR_LOGGING.  I think I experienced that bug while I was testing
my fix.

Anyway, to do it would require a few things, one is moving the
OutputDebugString call into _PUT_LOG, the other is storing and then mutating
buf[nb] so that only nb bytes are logged.

Comment 4

[timeless]

wtc: could you please review this?

Comment 5

[Wan-Teh Chang]

Attachment: alternative path v1.1

timeless, could you review and test this patch?

This is essentially your patch with some minor editings
and a change to use fwrite instead of fputs.

Comment 6

[timeless]

Comment on attachment 146815 [details] [diff] [review]
alternative path v1.1

works fine, looks fine (after reading man fwrite)
sorry about the \ formatting, i guess that's mozilla's style...
warning, the bit in @@ -348,8 +361,8 @@
will conflict with the patch in bug 237326. (since that bug rewrites the entire
function, but includes the same change.)

Comment 7

[Wan-Teh Chang]

Patch checked into the NSPR tip (NSPR 4.6) and
NSPRPUB_PRE_4_2_CLIENT_BRANCH (Mozilla 1.8 alpha).

Thanks for the patch, timeless.