Log Message
GC in the middle of JSObject::allocatePropertyStorage can cause badness https://bugs.webkit.org/show_bug.cgi?id=83839
Reviewed by Geoffrey Garen. * _javascript_Core.vcproj/_javascript_Core/_javascript_Core.def: * jit/JITStubs.cpp: Making changes to use the new return value of growPropertyStorage. (JSC::DEFINE_STUB_FUNCTION): * runtime/JSObject.cpp: (JSC::JSObject::growPropertyStorage): Renamed to more accurately reflect that we're growing our already-existing PropertyStorage. * runtime/JSObject.h: (JSObject): (JSC::JSObject::setPropertyStorage): "Atomically" sets the new property storage and the new structure so that we can be sure a GC never occurs when our Structure info is out of sync with our PropertyStorage. (JSC): (JSC::JSObject::putDirectInternal): Moved the check to see if we should allocate more backing store before the actual property insertion into the structure. (JSC::JSObject::putDirectWithoutTransition): Ditto. (JSC::JSObject::transitionTo): Ditto. * runtime/Structure.cpp: (JSC::Structure::suggestedNewPropertyStorageSize): Added to keep the resize policy for property backing stores contained within the Structure class. (JSC): * runtime/Structure.h: (JSC::Structure::shouldGrowPropertyStorage): Lets clients know if another insertion into the Structure would require resizing the property backing store so that they can preallocate the required storage. (Structure):
Modified Paths
- trunk/Source/_javascript_Core/ChangeLog
- trunk/Source/_javascript_Core/_javascript_Core.vcproj/_javascript_Core/_javascript_Core.def
- trunk/Source/_javascript_Core/jit/JITStubs.cpp
- trunk/Source/_javascript_Core/runtime/JSObject.cpp
- trunk/Source/_javascript_Core/runtime/JSObject.h
- trunk/Source/_javascript_Core/runtime/Structure.cpp
- trunk/Source/_javascript_Core/runtime/Structure.h
Diff
Modified: trunk/Source/_javascript_Core/ChangeLog (114254 => 114255)
--- trunk/Source/_javascript_Core/ChangeLog 2012-04-16 14:49:35 UTC (rev 114254)
+++ trunk/Source/_javascript_Core/ChangeLog 2012-04-16 15:10:08 UTC (rev 114255)
@@ -1,3 +1,37 @@
+2012-04-16 Mark Hahnenberg <mhahnenb...@apple.com>
+
+ GC in the middle of JSObject::allocatePropertyStorage can cause badness
+ https://bugs.webkit.org/show_bug.cgi?id=83839
+
+ Reviewed by Geoffrey Garen.
+
+ * _javascript_Core.vcproj/_javascript_Core/_javascript_Core.def:
+ * jit/JITStubs.cpp: Making changes to use the new return value of growPropertyStorage.
+ (JSC::DEFINE_STUB_FUNCTION):
+ * runtime/JSObject.cpp:
+ (JSC::JSObject::growPropertyStorage): Renamed to more accurately reflect that we're
+ growing our already-existing PropertyStorage.
+ * runtime/JSObject.h:
+ (JSObject):
+ (JSC::JSObject::setPropertyStorage): "Atomically" sets the new property storage
+ and the new structure so that we can be sure a GC never occurs when our Structure
+ info is out of sync with our PropertyStorage.
+ (JSC):
+ (JSC::JSObject::putDirectInternal): Moved the check to see if we should
+ allocate more backing store before the actual property insertion into
+ the structure.
+ (JSC::JSObject::putDirectWithoutTransition): Ditto.
+ (JSC::JSObject::transitionTo): Ditto.
+ * runtime/Structure.cpp:
+ (JSC::Structure::suggestedNewPropertyStorageSize): Added to keep the resize policy
+ for property backing stores contained within the Structure class.
+ (JSC):
+ * runtime/Structure.h:
+ (JSC::Structure::shouldGrowPropertyStorage): Lets clients know if another insertion
+ into the Structure would require resizing the property backing store so that they can
+ preallocate the required storage.
+ (Structure):
+
2012-04-13 Sheriff Bot <webkit.review....@gmail.com>
Unreviewed, rolling out r114185.
Modified: trunk/Source/_javascript_Core/_javascript_Core.vcproj/_javascript_Core/_javascript_Core.def (114254 => 114255)
--- trunk/Source/_javascript_Core/_javascript_Core.vcproj/_javascript_Core/_javascript_Core.def 2012-04-16 14:49:35 UTC (rev 114254)
+++ trunk/Source/_javascript_Core/_javascript_Core.vcproj/_javascript_Core/_javascript_Core.def 2012-04-16 15:10:08 UTC (rev 114255)
@@ -64,7 +64,6 @@
?addSlowCase@Identifier@JSC@@CA?AV?$PassRefPtr@VStringImpl@WTF@@@WTF@@PAVExecState@2@PAVStringImpl@4@@Z
?addSlowCase@Identifier@JSC@@CA?AV?$PassRefPtr@VStringImpl@WTF@@@WTF@@PAVJSGlobalData@2@PAVStringImpl@4@@Z
?addStaticGlobals@JSGlobalObject@JSC@@IAEXPAUGlobalPropertyInfo@12@H@Z
- ?allocatePropertyStorage@JSObject@JSC@@QAEXAAVJSGlobalData@2@II@Z
?allocateSlowCase@MarkedAllocator@JSC@@AAEPAXXZ
?append@StringBuilder@WTF@@QAEXPBEI@Z
?append@StringBuilder@WTF@@QAEXPB_WI@Z
@@ -220,6 +219,7 @@
?globalExec@JSGlobalObject@JSC@@QAEPAVExecState@2@XZ
?globalObjectCount@Heap@JSC@@QAEIXZ
?grow@HandleSet@JSC@@AAEXXZ
+ ?growPropertyStorage@JSObject@JSC@@QAEPAV?$WriteBarrierBase@W4Unknown@JSC@@@2@AAVJSGlobalData@2@II@Z
?hasInstance@JSObject@JSC@@SA_NPAV12@PAVExecState@2@VJSValue@2@2@Z
?hasProperty@JSObject@JSC@@QBE_NPAVExecState@2@ABVIdentifier@2@@Z
?hasProperty@JSObject@JSC@@QBE_NPAVExecState@2@I@Z
@@ -329,6 +329,7 @@
?stopProfiling@Profiler@JSC@@QAE?AV?$PassRefPtr@VProfile@JSC@@@WTF@@PAVExecState@2@ABVUString@2@@Z
?stopSampling@JSGlobalData@JSC@@QAEXXZ
?substringSharingImpl@UString@JSC@@QBE?AV12@II@Z
+ ?suggestedNewPropertyStorageSize@Structure@JSC@@QAEIXZ
?symbolTableGet@JSVariableObject@JSC@@IAE_NABVIdentifier@2@AAVPropertyDescriptor@2@@Z
?synthesizePrototype@JSValue@JSC@@QBEPAVJSObject@2@PAVExecState@2@@Z
?thisObject@DebuggerCallFrame@JSC@@QBEPAVJSObject@2@XZ
Modified: trunk/Source/_javascript_Core/jit/JITStubs.cpp (114254 => 114255)
--- trunk/Source/_javascript_Core/jit/JITStubs.cpp 2012-04-16 14:49:35 UTC (rev 114254)
+++ trunk/Source/_javascript_Core/jit/JITStubs.cpp 2012-04-16 15:10:08 UTC (rev 114255)
@@ -1498,7 +1498,9 @@
ASSERT(baseValue.isObject());
JSObject* base = asObject(baseValue);
- base->allocatePropertyStorage(*stackFrame.globalData, oldSize, newSize);
+ JSGlobalData& globalData = *stackFrame.globalData;
+ PropertyStorage newStorage = base->growPropertyStorage(globalData, oldSize, newSize);
+ base->setPropertyStorage(globalData, newStorage, newStructure);
return base;
}
Modified: trunk/Source/_javascript_Core/runtime/JSObject.cpp (114254 => 114255)
--- trunk/Source/_javascript_Core/runtime/JSObject.cpp 2012-04-16 14:49:35 UTC (rev 114254)
+++ trunk/Source/_javascript_Core/runtime/JSObject.cpp 2012-04-16 15:10:08 UTC (rev 114255)
@@ -546,7 +546,7 @@
return m_inheritorID.get();
}
-void JSObject::allocatePropertyStorage(JSGlobalData& globalData, size_t oldSize, size_t newSize)
+PropertyStorage JSObject::growPropertyStorage(JSGlobalData& globalData, size_t oldSize, size_t newSize)
{
ASSERT(newSize > oldSize);
@@ -574,7 +574,7 @@
}
ASSERT(newPropertyStorage);
- m_propertyStorage.set(globalData, this, newPropertyStorage);
+ return newPropertyStorage;
}
bool JSObject::getOwnPropertyDescriptor(JSObject* object, ExecState* exec, const Identifier& propertyName, PropertyDescriptor& descriptor)
Modified: trunk/Source/_javascript_Core/runtime/JSObject.h (114254 => 114255)
--- trunk/Source/_javascript_Core/runtime/JSObject.h 2012-04-16 14:49:35 UTC (rev 114254)
+++ trunk/Source/_javascript_Core/runtime/JSObject.h 2012-04-16 15:10:08 UTC (rev 114255)
@@ -212,8 +212,9 @@
bool staticFunctionsReified() { return structure()->staticFunctionsReified(); }
void reifyStaticFunctionsForDelete(ExecState* exec);
- JS_EXPORT_PRIVATE void allocatePropertyStorage(JSGlobalData&, size_t oldSize, size_t newSize);
+ JS_EXPORT_PRIVATE PropertyStorage growPropertyStorage(JSGlobalData&, size_t oldSize, size_t newSize);
bool isUsingInlineStorage() const { return static_cast<const void*>(m_propertyStorage.get()) == static_cast<const void*>(this + 1); }
+ void setPropertyStorage(JSGlobalData&, PropertyStorage, Structure*);
void* addressOfPropertyStorage()
{
@@ -447,6 +448,14 @@
return structure()->typeInfo().type() == GlobalThisType;
}
+inline void JSObject::setPropertyStorage(JSGlobalData& globalData, PropertyStorage storage, Structure* structure)
+{
+ ASSERT(storage);
+ ASSERT(structure);
+ setStructure(globalData, structure);
+ m_propertyStorage.set(globalData, this, storage);
+}
+
inline JSObject* constructEmptyObject(ExecState* exec, Structure* structure)
{
return JSFinalObject::create(exec, structure);
@@ -658,10 +667,11 @@
if ((mode == PutModePut) && !isExtensible())
return false;
- size_t currentCapacity = structure()->propertyStorageCapacity();
+ PropertyStorage newStorage = propertyStorage();
+ if (structure()->shouldGrowPropertyStorage())
+ newStorage = growPropertyStorage(globalData, structure()->propertyStorageCapacity(), structure()->suggestedNewPropertyStorageSize());
offset = structure()->addPropertyWithoutTransition(globalData, propertyName, attributes, specificFunction);
- if (currentCapacity != structure()->propertyStorageCapacity())
- allocatePropertyStorage(globalData, currentCapacity, structure()->propertyStorageCapacity());
+ setPropertyStorage(globalData, newStorage, structure());
ASSERT(offset < structure()->propertyStorageCapacity());
putDirectOffset(globalData, offset, value);
@@ -673,12 +683,13 @@
size_t offset;
size_t currentCapacity = structure()->propertyStorageCapacity();
- if (Structure* structure = Structure::addPropertyTransitionToExistingStructure(this->structure(), propertyName, attributes, specificFunction, offset)) {
+ if (Structure* structure = Structure::addPropertyTransitionToExistingStructure(this->structure(), propertyName, attributes, specificFunction, offset)) {
+ PropertyStorage newStorage = propertyStorage();
if (currentCapacity != structure->propertyStorageCapacity())
- allocatePropertyStorage(globalData, currentCapacity, structure->propertyStorageCapacity());
+ newStorage = growPropertyStorage(globalData, currentCapacity, structure->propertyStorageCapacity());
ASSERT(offset < structure->propertyStorageCapacity());
- setStructure(globalData, structure);
+ setPropertyStorage(globalData, newStorage, structure);
putDirectOffset(globalData, offset, value);
// This is a new property; transitions with specific values are not currently cachable,
// so leave the slot in an uncachable state.
@@ -722,13 +733,14 @@
if ((mode == PutModePut) && !isExtensible())
return false;
+ PropertyStorage newStorage = propertyStorage();
+ if (structure()->shouldGrowPropertyStorage())
+ newStorage = growPropertyStorage(globalData, structure()->propertyStorageCapacity(), structure()->suggestedNewPropertyStorageSize());
+
Structure* structure = Structure::addPropertyTransition(globalData, this->structure(), propertyName, attributes, specificFunction, offset);
- if (currentCapacity != structure->propertyStorageCapacity())
- allocatePropertyStorage(globalData, currentCapacity, structure->propertyStorageCapacity());
-
ASSERT(offset < structure->propertyStorageCapacity());
- setStructure(globalData, structure);
+ setPropertyStorage(globalData, newStorage, structure);
putDirectOffset(globalData, offset, value);
// This is a new property; transitions with specific values are not currently cachable,
// so leave the slot in an uncachable state.
@@ -762,18 +774,20 @@
inline void JSObject::putDirectWithoutTransition(JSGlobalData& globalData, const Identifier& propertyName, JSValue value, unsigned attributes)
{
ASSERT(!value.isGetterSetter() && !(attributes & Accessor));
- size_t currentCapacity = structure()->propertyStorageCapacity();
+ PropertyStorage newStorage = propertyStorage();
+ if (structure()->shouldGrowPropertyStorage())
+ newStorage = growPropertyStorage(globalData, structure()->propertyStorageCapacity(), structure()->suggestedNewPropertyStorageSize());
size_t offset = structure()->addPropertyWithoutTransition(globalData, propertyName, attributes, getJSFunction(value));
- if (currentCapacity != structure()->propertyStorageCapacity())
- allocatePropertyStorage(globalData, currentCapacity, structure()->propertyStorageCapacity());
+ setPropertyStorage(globalData, newStorage, structure());
putDirectOffset(globalData, offset, value);
}
inline void JSObject::transitionTo(JSGlobalData& globalData, Structure* newStructure)
{
+ PropertyStorage newStorage = propertyStorage();
if (structure()->propertyStorageCapacity() != newStructure->propertyStorageCapacity())
- allocatePropertyStorage(globalData, structure()->propertyStorageCapacity(), newStructure->propertyStorageCapacity());
- setStructure(globalData, newStructure);
+ newStorage = growPropertyStorage(globalData, structure()->propertyStorageCapacity(), newStructure->propertyStorageCapacity());
+ setPropertyStorage(globalData, newStorage, newStructure);
}
inline JSValue JSObject::toPrimitive(ExecState* exec, PreferredPrimitiveType preferredType) const
Modified: trunk/Source/_javascript_Core/runtime/Structure.cpp (114254 => 114255)
--- trunk/Source/_javascript_Core/runtime/Structure.cpp 2012-04-16 14:49:35 UTC (rev 114254)
+++ trunk/Source/_javascript_Core/runtime/Structure.cpp 2012-04-16 15:10:08 UTC (rev 114255)
@@ -267,6 +267,13 @@
m_propertyStorageCapacity *= 2;
}
+size_t Structure::suggestedNewPropertyStorageSize()
+{
+ if (isUsingInlineStorage())
+ return JSObject::baseExternalStorageCapacity;
+ return m_propertyStorageCapacity * 2;
+}
+
void Structure::despecifyDictionaryFunction(JSGlobalData& globalData, const Identifier& propertyName)
{
StringImpl* rep = propertyName.impl();
Modified: trunk/Source/_javascript_Core/runtime/Structure.h (114254 => 114255)
--- trunk/Source/_javascript_Core/runtime/Structure.h 2012-04-16 14:49:35 UTC (rev 114254)
+++ trunk/Source/_javascript_Core/runtime/Structure.h 2012-04-16 15:10:08 UTC (rev 114255)
@@ -101,6 +101,8 @@
bool isFrozen(JSGlobalData&);
bool isExtensible() const { return !m_preventExtensions; }
bool didTransition() const { return m_didTransition; }
+ bool shouldGrowPropertyStorage() { return propertyStorageCapacity() == propertyStorageSize(); }
+ JS_EXPORT_PRIVATE size_t suggestedNewPropertyStorageSize();
Structure* flattenDictionaryStructure(JSGlobalData&, JSObject*);
_______________________________________________ webkit-changes mailing list webkit-changes@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-changes
