Changeset 32222 in webkit for trunk/JavaScriptCore/kjs/ustring.cpp

Timestamp:

Apr 18, 2008, 12:46:13 PM (17 years ago)

Author:

[email protected]

Message:

Reviewed by Darin.

Fix leaks during plugin tests (which actually excercise background JS), and potential
PAC brokenness that was not reported, but very likely.

The leaks shadowed a bigger problem with Identifier destruction. Identifier::remove involves
an IdentifierTable lookup, which is now a per-thread instance. Since garbage collection can
currently happen on a different thread than allocation, a wrong table was used.

No measurable change on SunSpider total, ~1% variation on individual tests.

• kjs/ustring.cpp: (KJS::): (KJS::UString::Rep::create): (KJS::UString::Rep::destroy):
• kjs/ustring.h: Replaced isIdentifier with a pointer to IdentifierTable, so that destruction can be done correctly. Took one bit from reportedCost, to avoid making UString::Rep larger (performance effect was measurable on SunSpider).

• kjs/identifier.cpp: (KJS::IdentifierTable::IdentifierTable): (KJS::IdentifierTable::~IdentifierTable): (KJS::IdentifierTable::add): (KJS::IdentifierTable::remove): Make IdentifierTable a real class. Its destructor needs to zero out outstanding references, because some identifiers may briefly outlive it during thread destruction, and we don't want them to use their stale pointers.

(KJS::LiteralIdentifierTable):
(KJS::Identifier::add):
Now that LiteralIdentifierTable is per-thread and can be destroyed not just during application
shutdown, it is not appropriate to simply bump refcount for strings that get there; changed
the table to hold RefPtrs.

(KJS::CStringTranslator::translate):
(KJS::UCharBufferTranslator::translate):
(KJS::Identifier::addSlowCase):
(KJS::Identifier::remove):

• kjs/identifier.h: (KJS::Identifier::add): Use and update UString::Rep::identifierTable as appropriate. Updating it is now done in IdentifierTable::add, not in translators.

File:

• trunk/JavaScriptCore/kjs/ustring.cpp (modified) (4 diffs)

trunk/JavaScriptCore/kjs/ustring.cpp

@@ -174 +174 @@
 // These static strings are immutable, except for rc, whose initial value is chosen to reduce the possibility of it becoming zero due to ref/deref not being thread-safe.
 static UChar sharedEmptyChar;
-UString::Rep UString::Rep::null = { 0, 0, INT_MAX / 2, 0, false, true, &UString::Rep::null, 0, 0, 0, 0, 0, 0 };
-UString::Rep UString::Rep::empty = { 0, 0, INT_MAX / 2, 0, false, true, &UString::Rep::empty, 0, &sharedEmptyChar, 0, 0, 0, 0 };
+UString::Rep UString::Rep::null = { 0, 0, INT_MAX / 2, 0, 0, &UString::Rep::null, true, 0, 0, 0, 0, 0, 0 };
+UString::Rep UString::Rep::empty = { 0, 0, INT_MAX / 2, 0, 0, &UString::Rep::empty, true, 0, &sharedEmptyChar, 0, 0, 0, 0 };
 
 static char* statBuffer = 0; // Only used for debugging via UString::ascii().
@@ -195 +195 @@
   r->rc = 1;
   r->_hash = 0;
-  r->isIdentifier = false;
+  r->identifierTable = 0;
+  r->baseString = r;
   r->isStatic = false;
-  r->baseString = r;
   r->reportedCost = 0;
   r->buf = d;
@@ -225 +225 @@
   r->rc = 1;
   r->_hash = 0;
-  r->isIdentifier = false;
+  r->identifierTable = 0;
+  r->baseString = base.releaseRef();
   r->isStatic = false;
-  r->baseString = base.releaseRef();
   r->reportedCost = 0;
   r->buf = 0;
@@ -243 +243 @@
   // Static null and empty strings can never be destroyed, but we cannot rely on reference counting, because ref/deref are not thread-safe.
   if (!isStatic) {
-    if (isIdentifier)
+    if (identifierTable)
       Identifier::remove(this);
     if (baseString == this)