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

Status: RESOLVED FIXED

(Core :: XPCOM, enhancement)

Description

[Yaron Tausky [:ytausky]]

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.

Comment 1

[Yaron Tausky [:ytausky]]

Attachment: Bug 1508627: Remove dangerous assertion from nsCOMPtr

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.

Comment 2

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/dbb1558cb40e
Remove dangerous assertion from nsCOMPtr r=froydnj

Comment 3

[Cristina Coroiu [:ccoroiu]]

https://hg.mozilla.org/mozilla-central/rev/dbb1558cb40e