Bug 22887 – Make UString::Rep use RefCounted rather than implementing its own ref counting

• Status: RESOLVED
• Resolution: INVALID
• Component: JavaScriptCore

[Cameron Zwarich (cpst)]

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.

[Cameron Zwarich (cpst)]

Changing bug title to reflect the true purpose of this bug.

[Cameron Zwarich (cpst)]

Some preparatory work for fixing this was landed in r39350:

http://trac.webkit.org/changeset/39350

[Cameron Zwarich (cpst)]

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?

[Darin Adler]

(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.

[Gavin Barraclough]

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.