Commit ad21fd2f authored by mhahnenberg@apple.com's avatar mhahnenberg@apple.com

opaqueJSClassData should be cached on JSGlobalObject, not the JSGlobalData

https://bugs.webkit.org/show_bug.cgi?id=113086

Reviewed by Geoffrey Garen.

opaqueJSClassData stores cached prototypes for JSClassRefs in the C API. It doesn't make sense to 
share these prototypes within a JSGlobalData across JSGlobalObjects, and in fact doing so will cause 
a leak of the original JSGlobalObject that these prototypes were created in. Therefore we should move 
this cache to JSGlobalObject where it belongs and where it won't cause memory leaks.

* API/JSBase.cpp: Needed to add an extern "C" so that testapi.c can use the super secret GC function.
* API/JSClassRef.cpp: We now grab the cached context data from the global object rather than the global data.
(OpaqueJSClass::contextData):
* API/JSClassRef.h: Remove this header because it's unnecessary and causes circular dependencies.
* API/tests/testapi.c: Added a new test that makes sure that using the same JSClassRef in two different contexts
doesn't cause leaks of the original global object.
(leakFinalize):
(nestedAllocateObject): This is a hack to bypass the conservative scan of the GC, which was unnecessarily marking
objects and keeping them alive, ruining the test result.
(testLeakingPrototypesAcrossContexts):
(main):
* API/tests/testapi.mm: extern "C" this so we can continue using it here.
* runtime/JSGlobalData.cpp: Remove JSClassRef related stuff.
(JSC::JSGlobalData::~JSGlobalData):
* runtime/JSGlobalData.h:
(JSGlobalData):
* runtime/JSGlobalObject.h: Add the stuff that JSGlobalData had. We add it to JSGlobalObjectRareData so that 
clients who don't use the C API don't have to pay the memory cost of this extra HashMap.
(JSGlobalObject):
(JSGlobalObjectRareData):
(JSC::JSGlobalObject::opaqueJSClassData):


git-svn-id: http://svn.webkit.org/repository/webkit/trunk@146682 268f45cc-cd09-0410-ab3c-d52691b4dbfc
parent 675e03dd
...@@ -111,7 +111,7 @@ void JSReportExtraMemoryCost(JSContextRef ctx, size_t size) ...@@ -111,7 +111,7 @@ void JSReportExtraMemoryCost(JSContextRef ctx, size_t size)
exec->globalData().heap.reportExtraMemoryCost(size); exec->globalData().heap.reportExtraMemoryCost(size);
} }
JS_EXPORT void JSSynchronousGarbageCollectForDebugging(JSContextRef); extern "C" JS_EXPORT void JSSynchronousGarbageCollectForDebugging(JSContextRef);
void JSSynchronousGarbageCollectForDebugging(JSContextRef ctx) void JSSynchronousGarbageCollectForDebugging(JSContextRef ctx)
{ {
......
...@@ -151,7 +151,7 @@ OpaqueJSClassContextData::OpaqueJSClassContextData(JSC::JSGlobalData&, OpaqueJSC ...@@ -151,7 +151,7 @@ OpaqueJSClassContextData::OpaqueJSClassContextData(JSC::JSGlobalData&, OpaqueJSC
OpaqueJSClassContextData& OpaqueJSClass::contextData(ExecState* exec) OpaqueJSClassContextData& OpaqueJSClass::contextData(ExecState* exec)
{ {
OwnPtr<OpaqueJSClassContextData>& contextData = exec->globalData().opaqueJSClassData.add(this, nullptr).iterator->value; OwnPtr<OpaqueJSClassContextData>& contextData = exec->lexicalGlobalObject()->opaqueJSClassData().add(this, nullptr).iterator->value;
if (!contextData) if (!contextData)
contextData = adoptPtr(new OpaqueJSClassContextData(exec->globalData(), this)); contextData = adoptPtr(new OpaqueJSClassContextData(exec->globalData(), this));
return *contextData; return *contextData;
......
...@@ -26,10 +26,9 @@ ...@@ -26,10 +26,9 @@
#ifndef JSClassRef_h #ifndef JSClassRef_h
#define JSClassRef_h #define JSClassRef_h
#include "JSObjectRef.h" #include <JavaScriptCore/JSObjectRef.h>
#include "Weak.h" #include "Weak.h"
#include "JSObject.h"
#include "Protect.h" #include "Protect.h"
#include <wtf/HashMap.h> #include <wtf/HashMap.h>
#include <wtf/text/WTFString.h> #include <wtf/text/WTFString.h>
...@@ -87,7 +86,7 @@ public: ...@@ -87,7 +86,7 @@ public:
struct OpaqueJSClass : public ThreadSafeRefCounted<OpaqueJSClass> { struct OpaqueJSClass : public ThreadSafeRefCounted<OpaqueJSClass> {
static PassRefPtr<OpaqueJSClass> create(const JSClassDefinition*); static PassRefPtr<OpaqueJSClass> create(const JSClassDefinition*);
static PassRefPtr<OpaqueJSClass> createNoAutomaticPrototype(const JSClassDefinition*); static PassRefPtr<OpaqueJSClass> createNoAutomaticPrototype(const JSClassDefinition*);
~OpaqueJSClass(); JS_EXPORT_PRIVATE ~OpaqueJSClass();
String className(); String className();
OpaqueJSClassStaticValuesTable* staticValues(JSC::ExecState*); OpaqueJSClassStaticValuesTable* staticValues(JSC::ExecState*);
......
...@@ -55,6 +55,8 @@ using std::isnan; ...@@ -55,6 +55,8 @@ using std::isnan;
void testObjectiveCAPI(void); void testObjectiveCAPI(void);
#endif #endif
extern void JSSynchronousGarbageCollectForDebugging(JSContextRef);
static JSGlobalContextRef context; static JSGlobalContextRef context;
int failed; int failed;
static void assertEqualsAsBoolean(JSValueRef value, bool expectedValue) static void assertEqualsAsBoolean(JSValueRef value, bool expectedValue)
...@@ -132,6 +134,61 @@ static void assertEqualsAsCharactersPtr(JSValueRef value, const char* expectedVa ...@@ -132,6 +134,61 @@ static void assertEqualsAsCharactersPtr(JSValueRef value, const char* expectedVa
JSStringRelease(valueAsString); JSStringRelease(valueAsString);
} }
static int leakedObject = 1;
static void leakFinalize(JSObjectRef object)
{
(void)object;
leakedObject = 0;
}
// This is a hack to avoid the C++ stack keeping the original JSObject alive.
static void nestedAllocateObject(JSContextRef context, JSClassRef class, unsigned n)
{
if (!n) {
JSObjectRef object = JSObjectMake(context, class, 0);
JSObjectRef globalObject = JSContextGetGlobalObject(context);
JSStringRef propertyName = JSStringCreateWithUTF8CString("value");
JSObjectSetProperty(context, globalObject, propertyName, object, kJSPropertyAttributeNone, 0);
JSStringRelease(propertyName);
return;
}
nestedAllocateObject(context, class, n - 1);
}
static void testLeakingPrototypesAcrossContexts()
{
JSClassDefinition leakDefinition = kJSClassDefinitionEmpty;
leakDefinition.finalize = leakFinalize;
JSClassRef leakClass = JSClassCreate(&leakDefinition);
JSContextGroupRef group = JSContextGroupCreate();
{
JSGlobalContextRef context1 = JSGlobalContextCreateInGroup(group, NULL);
nestedAllocateObject(context1, leakClass, 10);
JSGlobalContextRelease(context1);
}
{
JSGlobalContextRef context2 = JSGlobalContextCreateInGroup(group, NULL);
JSObjectRef object2 = JSObjectMake(context2, leakClass, 0);
JSValueProtect(context2, object2);
JSSynchronousGarbageCollectForDebugging(context2);
if (leakedObject) {
printf("FAIL: Failed to finalize the original object after the first GC.\n");
failed = 1;
} else
printf("PASS: Finalized the original object as expected.\n");
JSValueUnprotect(context2, object2);
JSGlobalContextRelease(context2);
}
JSContextGroupRelease(group);
JSClassRelease(leakClass);
}
static bool timeZoneIsPST() static bool timeZoneIsPST()
{ {
char timeZoneName[70]; char timeZoneName[70];
...@@ -1685,6 +1742,8 @@ int main(int argc, char* argv[]) ...@@ -1685,6 +1742,8 @@ int main(int argc, char* argv[])
free(scriptUTF8); free(scriptUTF8);
} }
testLeakingPrototypesAcrossContexts();
// Clear out local variables pointing at JSObjectRefs to allow their values to be collected // Clear out local variables pointing at JSObjectRefs to allow their values to be collected
function = NULL; function = NULL;
v = NULL; v = NULL;
......
...@@ -25,7 +25,7 @@ ...@@ -25,7 +25,7 @@
#import <JavaScriptCore/JavaScriptCore.h> #import <JavaScriptCore/JavaScriptCore.h>
void JSSynchronousGarbageCollectForDebugging(JSContextRef); extern "C" void JSSynchronousGarbageCollectForDebugging(JSContextRef);
extern "C" bool _Block_has_signature(id); extern "C" bool _Block_has_signature(id);
extern "C" const char * _Block_signature(id); extern "C" const char * _Block_signature(id);
......
2013-03-22 Mark Hahnenberg <mhahnenberg@apple.com>
opaqueJSClassData should be cached on JSGlobalObject, not the JSGlobalData
https://bugs.webkit.org/show_bug.cgi?id=113086
Reviewed by Geoffrey Garen.
opaqueJSClassData stores cached prototypes for JSClassRefs in the C API. It doesn't make sense to
share these prototypes within a JSGlobalData across JSGlobalObjects, and in fact doing so will cause
a leak of the original JSGlobalObject that these prototypes were created in. Therefore we should move
this cache to JSGlobalObject where it belongs and where it won't cause memory leaks.
* API/JSBase.cpp: Needed to add an extern "C" so that testapi.c can use the super secret GC function.
* API/JSClassRef.cpp: We now grab the cached context data from the global object rather than the global data.
(OpaqueJSClass::contextData):
* API/JSClassRef.h: Remove this header because it's unnecessary and causes circular dependencies.
* API/tests/testapi.c: Added a new test that makes sure that using the same JSClassRef in two different contexts
doesn't cause leaks of the original global object.
(leakFinalize):
(nestedAllocateObject): This is a hack to bypass the conservative scan of the GC, which was unnecessarily marking
objects and keeping them alive, ruining the test result.
(testLeakingPrototypesAcrossContexts):
(main):
* API/tests/testapi.mm: extern "C" this so we can continue using it here.
* runtime/JSGlobalData.cpp: Remove JSClassRef related stuff.
(JSC::JSGlobalData::~JSGlobalData):
* runtime/JSGlobalData.h:
(JSGlobalData):
* runtime/JSGlobalObject.h: Add the stuff that JSGlobalData had. We add it to JSGlobalObjectRareData so that
clients who don't use the C API don't have to pay the memory cost of this extra HashMap.
(JSGlobalObject):
(JSGlobalObjectRareData):
(JSC::JSGlobalObject::opaqueJSClassData):
2013-03-19 Martin Robinson <mrobinson@igalia.com> 2013-03-19 Martin Robinson <mrobinson@igalia.com>
[GTK] Add support for building the WebCore bindings to the gyp build [GTK] Add support for building the WebCore bindings to the gyp build
......
...@@ -742,7 +742,7 @@ ...@@ -742,7 +742,7 @@
BC18C41A0E16F5CD00B34460 /* JSCallbackFunction.h in Headers */ = {isa = PBXBuildFile; fileRef = 1440F88F0A508B100005F061 /* JSCallbackFunction.h */; }; BC18C41A0E16F5CD00B34460 /* JSCallbackFunction.h in Headers */ = {isa = PBXBuildFile; fileRef = 1440F88F0A508B100005F061 /* JSCallbackFunction.h */; };
BC18C41B0E16F5CD00B34460 /* JSCallbackObject.h in Headers */ = {isa = PBXBuildFile; fileRef = 14ABDF5D0A437FEF00ECCA01 /* JSCallbackObject.h */; }; BC18C41B0E16F5CD00B34460 /* JSCallbackObject.h in Headers */ = {isa = PBXBuildFile; fileRef = 14ABDF5D0A437FEF00ECCA01 /* JSCallbackObject.h */; };
BC18C41C0E16F5CD00B34460 /* JSCallbackObjectFunctions.h in Headers */ = {isa = PBXBuildFile; fileRef = A8E894310CD0602400367179 /* JSCallbackObjectFunctions.h */; }; BC18C41C0E16F5CD00B34460 /* JSCallbackObjectFunctions.h in Headers */ = {isa = PBXBuildFile; fileRef = A8E894310CD0602400367179 /* JSCallbackObjectFunctions.h */; };
BC18C41D0E16F5CD00B34460 /* JSClassRef.h in Headers */ = {isa = PBXBuildFile; fileRef = 1440FCE10A51E46B0005F061 /* JSClassRef.h */; }; BC18C41D0E16F5CD00B34460 /* JSClassRef.h in Headers */ = {isa = PBXBuildFile; fileRef = 1440FCE10A51E46B0005F061 /* JSClassRef.h */; settings = {ATTRIBUTES = (Private, ); }; };
BC18C41E0E16F5CD00B34460 /* JSContextRef.h in Headers */ = {isa = PBXBuildFile; fileRef = 14BD5A2A0A3E91F600BAF59C /* JSContextRef.h */; settings = {ATTRIBUTES = (Public, ); }; }; BC18C41E0E16F5CD00B34460 /* JSContextRef.h in Headers */ = {isa = PBXBuildFile; fileRef = 14BD5A2A0A3E91F600BAF59C /* JSContextRef.h */; settings = {ATTRIBUTES = (Public, ); }; };
BC18C41F0E16F5CD00B34460 /* JSFunction.h in Headers */ = {isa = PBXBuildFile; fileRef = F692A85F0255597D01FF60F7 /* JSFunction.h */; settings = {ATTRIBUTES = (Private, ); }; }; BC18C41F0E16F5CD00B34460 /* JSFunction.h in Headers */ = {isa = PBXBuildFile; fileRef = F692A85F0255597D01FF60F7 /* JSFunction.h */; settings = {ATTRIBUTES = (Private, ); }; };
BC18C4200E16F5CD00B34460 /* JSGlobalData.h in Headers */ = {isa = PBXBuildFile; fileRef = E18E3A560DF9278C00D90B34 /* JSGlobalData.h */; settings = {ATTRIBUTES = (Private, ); }; }; BC18C4200E16F5CD00B34460 /* JSGlobalData.h in Headers */ = {isa = PBXBuildFile; fileRef = E18E3A560DF9278C00D90B34 /* JSGlobalData.h */; settings = {ATTRIBUTES = (Private, ); }; };
...@@ -2956,6 +2956,7 @@ ...@@ -2956,6 +2956,7 @@
0FB7F39515ED8E4600F167B2 /* ArrayConventions.h in Headers */, 0FB7F39515ED8E4600F167B2 /* ArrayConventions.h in Headers */,
0F63945515D07057006A597C /* ArrayProfile.h in Headers */, 0F63945515D07057006A597C /* ArrayProfile.h in Headers */,
BC18C3E70E16F5CD00B34460 /* ArrayPrototype.h in Headers */, BC18C3E70E16F5CD00B34460 /* ArrayPrototype.h in Headers */,
BC18C41D0E16F5CD00B34460 /* JSClassRef.h in Headers */,
86E3C61D167BABEE006D760A /* JSVirtualMachineInternal.h in Headers */, 86E3C61D167BABEE006D760A /* JSVirtualMachineInternal.h in Headers */,
BC18C5240E16FC8A00B34460 /* ArrayPrototype.lut.h in Headers */, BC18C5240E16FC8A00B34460 /* ArrayPrototype.lut.h in Headers */,
86E3C617167BABEE006D760A /* JSContextInternal.h in Headers */, 86E3C617167BABEE006D760A /* JSContextInternal.h in Headers */,
...@@ -3154,7 +3155,6 @@ ...@@ -3154,7 +3155,6 @@
0F9749711687ADE400A4FF6A /* JSCellInlines.h in Headers */, 0F9749711687ADE400A4FF6A /* JSCellInlines.h in Headers */,
BC18C42B0E16F5CD00B34460 /* JSCJSValue.h in Headers */, BC18C42B0E16F5CD00B34460 /* JSCJSValue.h in Headers */,
865A30F1135007E100CDB49E /* JSCJSValueInlines.h in Headers */, 865A30F1135007E100CDB49E /* JSCJSValueInlines.h in Headers */,
BC18C41D0E16F5CD00B34460 /* JSClassRef.h in Headers */,
86E3C613167BABD7006D760A /* JSContext.h in Headers */, 86E3C613167BABD7006D760A /* JSContext.h in Headers */,
BC18C41E0E16F5CD00B34460 /* JSContextRef.h in Headers */, BC18C41E0E16F5CD00B34460 /* JSContextRef.h in Headers */,
148CD1D8108CF902008163C6 /* JSContextRefPrivate.h in Headers */, 148CD1D8108CF902008163C6 /* JSContextRefPrivate.h in Headers */,
......
...@@ -44,7 +44,6 @@ ...@@ -44,7 +44,6 @@
#include "JSActivation.h" #include "JSActivation.h"
#include "JSAPIValueWrapper.h" #include "JSAPIValueWrapper.h"
#include "JSArray.h" #include "JSArray.h"
#include "JSClassRef.h"
#include "JSFunction.h" #include "JSFunction.h"
#include "JSLock.h" #include "JSLock.h"
#include "JSNameScope.h" #include "JSNameScope.h"
...@@ -306,8 +305,6 @@ JSGlobalData::~JSGlobalData() ...@@ -306,8 +305,6 @@ JSGlobalData::~JSGlobalData()
fastDelete(const_cast<HashTable*>(regExpPrototypeTable)); fastDelete(const_cast<HashTable*>(regExpPrototypeTable));
fastDelete(const_cast<HashTable*>(stringConstructorTable)); fastDelete(const_cast<HashTable*>(stringConstructorTable));
opaqueJSClassData.clear();
delete emptyList; delete emptyList;
delete propertyNames; delete propertyNames;
......
...@@ -62,9 +62,6 @@ ...@@ -62,9 +62,6 @@
#include <wtf/ListHashSet.h> #include <wtf/ListHashSet.h>
#endif #endif
struct OpaqueJSClass;
struct OpaqueJSClassContextData;
namespace JSC { namespace JSC {
class CodeBlock; class CodeBlock;
...@@ -369,8 +366,6 @@ namespace JSC { ...@@ -369,8 +366,6 @@ namespace JSC {
void gatherConservativeRoots(ConservativeRoots&); void gatherConservativeRoots(ConservativeRoots&);
#endif #endif
HashMap<OpaqueJSClass*, OwnPtr<OpaqueJSClassContextData> > opaqueJSClassData;
JSGlobalObject* dynamicGlobalObject; JSGlobalObject* dynamicGlobalObject;
HashSet<JSObject*> stringRecursionCheckVisitedObjects; HashSet<JSObject*> stringRecursionCheckVisitedObjects;
......
...@@ -24,6 +24,7 @@ ...@@ -24,6 +24,7 @@
#include "ArrayAllocationProfile.h" #include "ArrayAllocationProfile.h"
#include "JSArray.h" #include "JSArray.h"
#include "JSClassRef.h"
#include "JSGlobalData.h" #include "JSGlobalData.h"
#include "JSSegmentedVariableObject.h" #include "JSSegmentedVariableObject.h"
#include "JSWeakObjectMapRefInternal.h" #include "JSWeakObjectMapRefInternal.h"
...@@ -38,6 +39,9 @@ ...@@ -38,6 +39,9 @@
#include <wtf/OwnPtr.h> #include <wtf/OwnPtr.h>
#include <wtf/RandomNumber.h> #include <wtf/RandomNumber.h>
struct OpaqueJSClass;
struct OpaqueJSClassContextData;
namespace JSC { namespace JSC {
class ArrayPrototype; class ArrayPrototype;
...@@ -86,6 +90,7 @@ struct GlobalObjectMethodTable { ...@@ -86,6 +90,7 @@ struct GlobalObjectMethodTable {
class JSGlobalObject : public JSSegmentedVariableObject { class JSGlobalObject : public JSSegmentedVariableObject {
private: private:
typedef HashSet<RefPtr<OpaqueJSWeakObjectMap> > WeakMapSet; typedef HashSet<RefPtr<OpaqueJSWeakObjectMap> > WeakMapSet;
typedef HashMap<OpaqueJSClass*, OwnPtr<OpaqueJSClassContextData> > OpaqueJSClassDataMap;
struct JSGlobalObjectRareData { struct JSGlobalObjectRareData {
JSGlobalObjectRareData() JSGlobalObjectRareData()
...@@ -95,6 +100,8 @@ private: ...@@ -95,6 +100,8 @@ private:
WeakMapSet weakMaps; WeakMapSet weakMaps;
unsigned profileGroup; unsigned profileGroup;
OpaqueJSClassDataMap opaqueJSClassData;
}; };
protected: protected:
...@@ -395,6 +402,12 @@ public: ...@@ -395,6 +402,12 @@ public:
m_rareData->weakMaps.remove(map); m_rareData->weakMaps.remove(map);
} }
OpaqueJSClassDataMap& opaqueJSClassData()
{
createRareDataIfNeeded();
return m_rareData->opaqueJSClassData;
}
double weakRandomNumber() { return m_weakRandom.get(); } double weakRandomNumber() { return m_weakRandom.get(); }
unsigned weakRandomInteger() { return m_weakRandom.getUint32(); } unsigned weakRandomInteger() { return m_weakRandom.getUint32(); }
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment