We're already working on a static analysis in
https://bugzilla.mozilla.org/show_bug.cgi?id=1180993 which will prevent
code like that from passing try.
Hopefully that will help with your concerns.
On 2015-07-07 12:59 PM, Seth Fowler wrote:
I brought this up on IRC, but I think this is worth taking to the list:
Replacing TemporaryRef with already_AddRefed has already caused at least one
leak because already_AddRefed does not call Release() on the pointer it holds
if nothing takes ownership of that pointer before its destructor runs.
TemporaryRef did do that, and this seems like a strictly safer approach.
My question is: why doesn’t already_AddRefed call Release() in its destructor?
The fact that it doesn’t seems to me to be a profoundly unintuitive footgun.
I don’t think we can trust reviewers to catch this, either. The fact that Foo()
is declared as:
already_AddRefed<Bar> Foo();
won’t necessarily be obvious from a call site, where the reviewer will just see:
Foo();
That call will leak, and if we don’t happen to hit that code path in our tests,
we may not find out about it until after shipping it.
I’d prefer that already_AddRefed just call Release(), but if there are
performance arguments against that, then we should be checking for this pattern
with static analysis. I don’t think the situation as it stands should be
allowed to remain for long.
- Seth
On Jun 30, 2015, at 12:02 PM, Nathan Froyd <nfr...@mozilla.com> wrote:
Bug 1161627 has landed on inbound, which converts all uses of
mozilla::TemporaryRef to already_AddRefed and removes TemporaryRef.
(already_AddRefed moved to MFBT several months ago, in case you were
wondering about the spreading of XPCOM concepts.) TemporaryRef was added
for easier porting of code from other engines, but as it has not been used
much for that purpose and it led to somewhat less efficient code in places,
it was deemed a failed experiment.
-Nathan
_______________________________________________
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform
_______________________________________________
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform
_______________________________________________
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform
