The default bug view has changed. See this FAQ.

remove cx argument from principals-related callbacks

RESOLVED FIXED

Status

()

Core
JavaScript Engine
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: Igor Bukanov, Assigned: Igor Bukanov)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 obsolete attachment)

(Assignee)

Description

5 years ago
As a leftover from the multithreaded runtime era JSPRINCIPALS_(HOLD|DROP) still use atomic operations when updating JSPrincipals::refcount. We should remove that. Also it would be nice to remove the unused JSContext *cx parameter that often requires to pass that cx through many layers of API.
(Assignee)

Comment 1

5 years ago
Created attachment 595181 [details] [diff] [review]
v1

The patch drops atomic operations around JSPrincipals::refcount and removes the cx parameter from  JSPRINCIPALS_(HOLD|DROP). That removal is propagated to the callers when possible.
Assignee: general → igor
Attachment #595181 - Flags: review?(mrbkap)
Is this safe given the comments near the end of bug 143559?  nsPrincipal uses atomic increment/decrement on the JSPrincipals, and could race with main-thread code per that bug, right?
(Assignee)

Comment 3

5 years ago
(In reply to Boris Zbarsky (:bz) from comment #2)
> Is this safe given the comments near the end of bug 143559?  nsPrincipal
> uses atomic increment/decrement on the JSPrincipals, and could race with
> main-thread code per that bug, right?

You are absolutely right - the principals must continue to use atomic refcounter mutators. So I mutate the bug to be about eliminating unnecessary cx parameter from principals-related methods. That allows to eliminate few useless compartment enter() calls.
Summary: remove multithreaded support from JSPrincipals → remove cx argument from principals-related callbacks
(Assignee)

Updated

5 years ago
Attachment #595181 - Attachment is obsolete: true
Attachment #595181 - Flags: review?(mrbkap)
(Assignee)

Comment 4

5 years ago
This was addressed in the bug 730221.
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Depends on: 730221
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.