TL;DR: returning nsRefPtr/RefPtr from functions is now safer, assuming
appropriate compiler support. Go forth and experiment with returning such
values, and comment at [4] if you have issues with passing nsRefPtr/RefPtr
return values into functions that expect raw pointers.
Hi all,
People have had concerns about returning reference-counted smart pointers
(RefPtr, nsRefPtr, nsCOMPtr) from functions due to easy-to-write but
hard-to spot bugs.
Last week, Aryeh Gregor landed patches[1][2] that prohibits the dangerous
cases for nsRefPtr and RefPtr if you are compiling with clang, GCC >=
4.8.1, or MSVC 2015. The related work for nsCOMPtr is happening in [3].
It is worth noting that not all of our infrastructure runs the requisite
versions of the compilers, however.
To elaborate on the potentially buggy behavior, the concern was with a
function like this:
nsRefPtr<T> MaybeDoSomething(...) { ... }
because the implicit conversion to T* behavior of our smart pointers can
lead to hard-to-find-or-notice bugs:
T* danglingPointer = MaybeDoSomething(...);
as the temporary smart pointer will be destroyed at the end of that
statement, leaving |danglingPointer| pointing at freed memory.
There is one wrinkle with the above patches. If you have MaybeDoSomething,
as above, and you have:
DoOtherThing(T* aThing);
you cannot write things like:
DoOtherThing(MaybeDoSomething(...));
DoOtherThing(condition ? mOfTypeT : nullptr);
you would need to write instead:
nsRefPtr<T> temporary = MaybeDoSomething(...);
DoOtherThing(temporary);
DoOtherThing(condition ? mOfTypeT.get() : nullptr);
// Other options are available, of course...
There were relatively few instances of code like the above, and most of
them were due to uses of the ternary ?: operator. It's an open question
how much more of a problem it will be as returning nsRefPtr/RefPtr becomes
more common.
A more complete solution was discussed in [1] and is the subject of [4] and
[5]. Please add your thoughts to [4] on how we might address the above
problem, or whether you think it is a problem at all.
Thanks,
-Nathan
[1]: https://bugzilla.mozilla.org/show_bug.cgi?id=1179451
[2]: https://bugzilla.mozilla.org/show_bug.cgi?id=1193298
[3]: https://bugzilla.mozilla.org/show_bug.cgi?id=1193762
[4]: https://bugzilla.mozilla.org/show_bug.cgi?id=1194195
[5]: https://bugzilla.mozilla.org/show_bug.cgi?id=1194215
_______________________________________________
dev-platform mailing list
[email protected]
https://lists.mozilla.org/listinfo/dev-platform
