450174 - Remove static string from nsDOMOfflineResourceList.cpp

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Dave Camp (:dcamp)]

As mentioned in bug 442806, the use of a static nsCAutoString could lead to a shutdown leak if we ever exceed an 80-character cached manifest.

Comment 1

[Serge Gautherie (:sgautherie)]

Bug 442806 comment 8:
{{
Boris Zbarsky (todo: 200+ items) [gone 8/9--8/17]   2008-08-07 19:56:55 PDT

>+static nsCAutoString gCachedManifestSpec;

So... generally this is bad: it'll show up as a leak any time the string value
is longer than 80 chars or whatever we use now.  Followup bug to not us a
static object here?
}}

It should be 64 (not 80).

***

IIRC, it was said that |static ns*String| (other than *Auto*) is even guaranteed to leak.
So the actual issue may be much wider than nsDOMOfflineResourceList.cpp only.

Comment 2

[Serge Gautherie (:sgautherie)]

(In reply to comment #1)
> IIRC, it was said that |static ns*String| (other than *Auto*) is even
> guaranteed to leak.
> So the actual issue may be much wider than nsDOMOfflineResourceList.cpp only.

I filed bug 451497 about finding out these issues by static analysis tool(s).

Comment 3

[Dave Camp (:dcamp)]

Attachment: make cached keys a member

Comment 4

[Dave Camp (:dcamp)]

Comment on attachment 335407 [details] [diff] [review]
make cached keys a member

Changes to the tests in bug 442806 expose this leak, so this should be fixed before we land that.

Comment 5

[Dave Camp (:dcamp)]

Pushed as http://hg.mozilla.org/mozilla-central/index.cgi/rev/b6fde4f6d35c