Closed
Bug 620251
Opened 14 years ago
Closed 13 years ago
js_CurrentThread and friends should have AndLockGC in their name
Categories
(Core :: JavaScript Engine, enhancement)
Core
JavaScript Engine
Tracking
()
RESOLVED
FIXED
People
(Reporter: timeless, Assigned: timeless)
References
(Blocks 1 open bug)
Details
(Keywords: coverity, Whiteboard: fixed-in-tracemonkey)
Attachments
(1 file)
|
(deleted),
patch
|
jorendorff
:
review+
|
Details | Diff | Splinter Review |
Some functions in spidermonkey are friendly and call out the fact that they're locking/unlocking:
DestroyTrapAndUnlock
DropWatchPointAndUnlock
the js_CurrentThread family is not friendly which results in poor comments being added after each call site:
785 #ifdef JS_THREADSAFE
786 if (!js_InitContextThread(cx)) {
787 FreeContext(cx);
788 return NULL;
789 }
790 #endif
791
792 /*
793 * Here the GC lock is still held after js_InitContextThread took it and
794 * the GC is not running on another thread.
5949 if (!js_InitContextThread(cx)) {
5950 js_ReportOutOfMemory(cx);
5951 return -1;
5952 }
5953
5954 /* Here the GC lock is still held after js_InitContextThread took it. */
While giving a function a helpful name doesn't immediately help a static analyzer, it does immediately help anyone using a static analyzer.
Summary: js_CurrentThread and friends should have AndLockGCRT or something in their name → js_CurrentThread and friends should have AndLockGC in their name
Assignee: general → timeless
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Attachment #498669 -
Flags: review?(jorendorff)
|
||
Comment 2•14 years ago
|
||
Comment on attachment 498669 [details] [diff] [review]
proposal
Review of attachment 498669 [details] [diff] [review]:
Absolutely. Righteous change. r=me.
Attachment #498669 -
Flags: review?(jorendorff) → review+
Keywords: checkin-needed
|
||
Comment 3•14 years ago
|
||
Un-bitrotted & landed: http://hg.mozilla.org/tracemonkey/rev/3277b0000889
Keywords: checkin-needed
Whiteboard: fixed-in-tracemonkey
|
||
Comment 4•13 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
|
||
Updated•6 years ago
|
Blocks: coverity-analysis
You need to log in
before you can comment on or make changes to this bug.
Description
•