Closed Bug 1508627 Opened 2 years ago Closed 2 years ago
COMPtr's move constructor and assignment operators unsafely touch the pointee's refcount in debug mode
47 bytes, text/x-phabricator-request
|Details | Review|
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.
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.
Pushed by email@example.com: https://hg.mozilla.org/integration/autoland/rev/dbb1558cb40e Remove dangerous assertion from nsCOMPtr r=froydnj
Assignee: nobody → ytausky
You need to log in before you can comment on or make changes to this bug.