Closed Bug 771512 Opened 12 years ago Closed 9 years ago

Add a static analysis to detect calls to functions which return an already_AddRefed, but don't use the return value

Categories

(Developer Infrastructure :: Source Code Analysis, defect)

Product:
Developer Infrastructure
Component:
Source Code Analysis
Platform:
x86
macOS
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(Not tracked)

Status:
RESOLVED DUPLICATE of bug 1180993

People

(Reporter: ehsan.akhgari, Unassigned)

Details

Those types of calls will make us leak a reference, and they should be disallowed.
somebody (khuey?) was working on a dynamic version of the same thing (asserting in the already_AddRefed destructor if the reference was not taken).
I was looking at it, but unfortunately it's hard :-/
In particular, the problem I ran into is that some things return already_AddRefed<T> where T is a class that pretends to be refcounted but really isn't.
(In reply to Kyle Huey [:khuey] (khuey@mozilla.com) from comment #3)
> In particular, the problem I ran into is that some things return
> already_AddRefed<T> where T is a class that pretends to be refcounted but
> really isn't.

Ew, we should make it an error to use it on a class that doesn't supports AddRef/Release.
To be clear, T has AddRef/Release, they just don't do anything, so if you don't do anything with the already_AddRefed there's no leak.
(In reply to comment #5)
> To be clear, T has AddRef/Release, they just don't do anything, so if you don't
> do anything with the already_AddRefed there's no leak.

That's a very bad pattern, I think...  Can you give a concrete example of where we do this?  Maybe there's a reason that I just don't see right now.
Permanent/static atoms have this pattern. I don't think it really affects this bug at all, and I can't see how it would really affect the dynamic analysis either.
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → DUPLICATE
Product: Core → Firefox Build System
Product: Firefox Build System → Developer Infrastructure
You need to log in before you can comment on or make changes to this bug.