Closed Bug 771512 Opened 13 years ago Closed 10 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: 10 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.