Closed Bug 400957 Opened 12 years ago Closed 12 years ago
Memory allocated for log
Buf in PR _Set Log Buffering is never freed .
Block in use (biu): Found block of size 16384 bytes at address 0x85168e0 (81.37% of total) At time of allocation, the call stack was:  PR_Malloc() at line 467 in "prmem.c"  PR_SetLogBuffering() at line 436 in "prlog.c"  _PR_InitLog() at line 256 in "prlog.c"  _PR_InitStuff() at line 241 in "prinit.c"  _PR_ImplicitInitialization() at line 259 in "prinit.c"  PR_GetSpecialFD() at line 1215 in "ptio.c"  main() at line 1006 in "ocspclnt.c"
Stacks in OPT version are a bit shorter, but still the same problem: selfserv/main/PR_Init/_PR_InitLog/PR_Malloc strsclnt/main/PR_Init/_PR_InitLog/PR_Malloc ocspclnt/main/PR_GetSpecialFD/_PR_ImplicitInitialization/_PR_InitLog/PR_Malloc
Thanks for the bug report, Slavo. I took the opportunity to verify that _PR_LogCleanup frees all the static variables in this file (_pr_logLock, logModules, logBuf, and logFile).
Attachment #286023 - Flags: review?(kengert)
Looking at the uses of logBuf, I see a potential crash. PR_SetLogBuffering does if (logBuf) PR_DELETE(logBuf); But it might not set a new buffer. I propose, whenever deleting logBuf, you should do logBuf = NULL; Should we the same = NULL in your patch, too? Should PR_SetLogBuffering use _PR_LOCK_LOG() / _PR_UNLOCK_LOG() ?
Good question, Kai. PR_DELETE is a macro -- it calls PR_Free and then sets the pointer to NULL. So it does what you suggested.
Comment on attachment 286023 [details] [diff] [review] Proposed patch: free logBuf in _PR_LogCleanup Thanks Wan-Teh. r=kengert
Attachment #286023 - Flags: review?(kengert) → review+
PR_SetLogBuffering and PR_SetLogFile should use _PR_LOCK_LOG() / _PR_UNLOCK_LOG(). However, we can't just enclose the functions' bodies with _PR_LOCK_LOG() / _PR_UNLOCK_LOG(). Specifically, we can't lock around calls to other NSPR functions (e.g., PR_Open) because they may perform logging, which would result in a deadlock. So the patch to fix this could be a little complicated. I suggest that we address the locking issue in a separate bug. I checked in my patch on the NSPR trunk for NSPR 4.7. Checking in prlog.c; /cvsroot/mozilla/nsprpub/pr/src/io/prlog.c,v <-- prlog.c new revision: 3.40; previous revision: 3.39 done
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → 4.7
You need to log in before you can comment on or make changes to this bug.