Bug 22887 - Make UString::Rep use RefCounted rather than implementing its own ref counting
Summary: Make UString::Rep use RefCounted rather than implementing its own ref counting
Status: RESOLVED INVALID
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Cameron Zwarich (cpst)
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2008-12-16 14:56 PST by Cameron Zwarich (cpst)
Modified: 2012-03-06 23:55 PST (History)
2 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Cameron Zwarich (cpst) 2008-12-16 14:56:45 PST
We should add assertions to UString::Rep::ref() and UString::Rep::deref(). This will make bugs like bug 22869 a lot easier to find, and it will also allow us to write reliable tests for them when we find them. It might even make sense to make UString::Rep use RefCounted like other ref counted objects.

The ref counts currently start at 1, so the only problem seems to be that the two Translators in Identiifer.cpp zero out the ref counts of newly created UString::Reps.
Comment 1 Cameron Zwarich (cpst) 2008-12-16 18:02:59 PST
Changing bug title to reflect the true purpose of this bug.
Comment 2 Cameron Zwarich (cpst) 2008-12-16 18:23:39 PST
Some preparatory work for fixing this was landed in r39350:

http://trac.webkit.org/changeset/39350
Comment 3 Cameron Zwarich (cpst) 2008-12-16 23:00:27 PST
There is an unfortunate comment in UString.h:

            int rc; // For null and empty static strings, this field does not reflect a correct count, because ref/deref are not thread-safe. A special case in destroy() guarantees that these do not get deleted.

I will either need to inherit from RefCountedBase and implement my own deref() to handle this, or make a template specialization for RefCounted<UString::Rep>. Which do you prefer?
Comment 4 Darin Adler 2008-12-17 09:58:36 PST
(In reply to comment #3)
> I will either need to inherit from RefCountedBase and implement my own deref()
> to handle this, or make a template specialization for RefCounted<UString::Rep>.
> Which do you prefer?

Inheriting is better, I think. Not sure. We might need to talk it over in person.
Comment 5 Gavin Barraclough 2012-03-06 23:55:03 PST
This is no longer a change we intend to make right now.  StringImpl now uses a non-standard ref-counting scheme, where  the low bit is reserved to track 'static' objects that will never be deallocated.  This bug is now invalid.