60697 - [meta] |static nsCOMPtr| considered harmful

Status: RESOLVED INACTIVE

(Developer Infrastructure :: Source Code Analysis, defect, P3)

Description

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

I think the string "static nsCOMPtr" should not occur in the Mozilla source
code. It can mean one of two things:

 * a static nsCOMPtr object, which implies a static ctor/dtor, which is against
the rules in
http://www.mozilla.org/hacking/portable-cpp.html#dont_use_static_constructors
and also probably causes problems with DLL unloading (if we ever turn that back on)

 * a function that returns an nsCOMPtr, which is evil, because if someone
assigns the result into a non-nsCOMPtr, they'll have a pointer to freed memory.
 already_addrefed<T> is preferred.

So, does anybody see any reason to keep any of the uses listed in:
http://www.mozilla.org/hacking/portable-cpp.html#dont_use_static_constructors ?
There really aren't as many as I expected...

Comment 1

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

The second URL in the comment above should have been
http://lxr.mozilla.org/seamonkey/search?string=static+*nsCOMPtr&regexp=on

Comment 2

[edburns]

I just posted a fix for one of these instances, to bug 59530.

Comment 3

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

I need to double-check the fix that went in for bug 60729 and continue the work
there...

Comment 4

[edburns]

Dbaron, any action on this?

Comment 5

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Not really.  But hopefully I'll get to it sometime soon.  Setting TM
optimistically, although if someone else wants to do it that's fine
with me.

Comment 6

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Reality check.  Moving out to 0.9.1.

Comment 7

[Judson Valeski]

per discussion w/ dougt, topembed minusing this bug.

Comment 8

[mmc]

Searching lxr with link in comment 1 returns no matching files.

Comment 9

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

But http://lxr.mozilla.org/seamonkey/search?string=static+nsCOMPtr does.  LXR
regexp searching is probably even more broken than usual.

Comment 10

[Asa Dotzler [:asa]]

dbaron, should this be on our radar for 1.8? If not, can you minus the
blocking-aviary1.1 flag? Thanks.

Comment 11

[Michael Kohler [:mkohler]]

This is a mass change. Every comment has "assigned-to-new" in it.

I didn't look through the bugs, so I'm sorry if I change a bug which shouldn't be changed. But I guess these bugs are just bugs that were once assigned and people forgot to change the Status back when unassigning.

Comment 12

[Sylvestre Ledru [:Sylvestre]]

David, 18 years after, does it still make sense?
We have still a few occurrences:
https://searchfox.org/mozilla-central/search?q=static+nsCOMPtr&path=

Comment 13

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

- the static destructor issue still exists (although some of the problems caused by static destructors no longer do), StaticRefPtr avoids it

- Returning nsCOMPtr has been improved by https://hg.mozilla.org/mozilla-central/rev/a0daadf6943ce11e9858cdcb7aacfd63dfc0ce39, though there may be other reasons already_AddRefed<T> may still be better

Comment 14

[Sylvestre Ledru [:Sylvestre]]

closing as we haven't been using it for a while