Closed
Bug 400957
Opened 17 years ago
Closed 17 years ago
Memory allocated for logBuf in PR_SetLogBuffering is never freed.
Categories
(NSPR :: NSPR, defect)
NSPR
NSPR
Tracking
(Not tracked)
RESOLVED
FIXED
4.7
People
(Reporter: slavomir.katuscak+mozilla, Assigned: wtc)
Details
(Keywords: memory-leak)
Attachments
(1 file)
1.11 KB,
patch
|
KaiE
:
review+
|
Details | Diff | Splinter Review |
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: [1] PR_Malloc() at line 467 in "prmem.c" [2] PR_SetLogBuffering() at line 436 in "prlog.c" [3] _PR_InitLog() at line 256 in "prlog.c" [4] _PR_InitStuff() at line 241 in "prinit.c" [5] _PR_ImplicitInitialization() at line 259 in "prinit.c" [6] PR_GetSpecialFD() at line 1215 in "ptio.c" [7] main() at line 1006 in "ocspclnt.c"
Reporter | ||
Comment 1•17 years ago
|
||
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
Assignee | ||
Comment 2•17 years ago
|
||
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)
Comment 3•17 years ago
|
||
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() ?
Assignee | ||
Comment 4•17 years ago
|
||
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 5•17 years ago
|
||
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+
Assignee | ||
Comment 6•17 years ago
|
||
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: 17 years ago
Resolution: --- → FIXED
Target Milestone: --- → 4.7
You need to log in
before you can comment on or make changes to this bug.
Description
•