Closed Bug 620251 Opened 9 years ago Closed 9 years ago

js_CurrentThread and friends should have AndLockGC in their name

Categories

(Core :: JavaScript Engine, enhancement)

enhancement
Not set

Tracking

()

RESOLVED FIXED

People

(Reporter: timeless, Assigned: timeless)

References

(Blocks 1 open bug)

Details

(Keywords: coverity, Whiteboard: fixed-in-tracemonkey)

Attachments

(1 file)

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
Attached patch proposalSplinter Review
Assignee: general → timeless
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Attachment #498669 - Flags: review?(jorendorff)
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
http://hg.mozilla.org/mozilla-central/rev/3277b0000889
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.