Closed Bug 1508627 Opened Last year Closed Last year

nsCOMPtr's move constructor and assignment operators unsafely touch the pointee's refcount in debug mode

Categories

(Core :: XPCOM, enhancement)

enhancement
Not set

Tracking

()

RESOLVED FIXED
mozilla65
Tracking Status
firefox65 --- fixed

People

(Reporter: ytausky, Assigned: ytausky)

References

Details

Attachments

(1 file)

Both call NSCAP_ASSERT_NO_QUERY_NEEDED() even though there is no need for it (see the comment in nsCOMPtr's swap()). This causes problems since an nsCOMPtr should be movable in any thread, whereas the refcount might not be thread safe at all.

I think the proper solution here would be to remove those calls, since the two functions do not do any kind of conversion and thus the same argument as with swap() applies to them as well.
Blocks: 1507479
Move constructors and assignment operators are expected to be safe and
infallible. In debug mode, nsCOMPtr's move functions called a test function
that modified the pointee's refcount -- a potentially thread unsafe operation
that also opened the door to assertion failures if the pointee implemented
some access checks in AddRef() or Release(). This commit removes this function
call in those two functions.
Keywords: checkin-needed
Pushed by csabou@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/dbb1558cb40e
Remove dangerous assertion from nsCOMPtr r=froydnj
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/dbb1558cb40e
Status: NEW → RESOLVED
Closed: Last year
Resolution: --- → FIXED
Target Milestone: --- → mozilla65
You need to log in before you can comment on or make changes to this bug.