2011-03-18 Geoffrey Garen <ggaren@apple.com>
authorggaren@apple.com <ggaren@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 18 Mar 2011 23:07:16 +0000 (23:07 +0000)
committerggaren@apple.com <ggaren@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 18 Mar 2011 23:07:16 +0000 (23:07 +0000)
        Reviewed by Oliver Hunt.

        [GTK] JSC crashes in 32bit Release bots after r80743
        https://bugs.webkit.org/show_bug.cgi?id=56180

        The crash was caused by referencing GC memory from a GC destructor. This
        is not safe because destruction time / order is not guaranteed.

        * profiler/ProfileGenerator.cpp:
        (JSC::ProfileGenerator::create):
        (JSC::ProfileGenerator::ProfileGenerator):
        (JSC::ProfileGenerator::willExecute):
        (JSC::ProfileGenerator::didExecute):
        * profiler/ProfileGenerator.h:
        (JSC::ProfileGenerator::origin): Made ExecState* the first argument,
        to match the rest of this class and JSC.

        Use a JSGlobalObject* instead of an ExecState* with an indirect reference
        to a JSGlobalObject* to track our origin. This is simpler and more
        efficient, and it removes the destruction order dependency that was causing
        our crash.

        * profiler/Profiler.cpp:
        (JSC::Profiler::startProfiling): Updated for change to JSGlobalObject*.
        (JSC::Profiler::stopProfiling): New function for stopping all profiles
        for a given global object. This is more straight-forward than multiplexing
        through the old function.

        (JSC::dispatchFunctionToProfiles): Updated for change to JSGlobalObject*.
        * profiler/Profiler.h: Ditto.

        * runtime/JSGlobalObject.cpp:
        (JSC::JSGlobalObject::~JSGlobalObject): Ditto.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@81525 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/profiler/ProfileGenerator.cpp
Source/JavaScriptCore/profiler/ProfileGenerator.h
Source/JavaScriptCore/profiler/Profiler.cpp
Source/JavaScriptCore/profiler/Profiler.h
Source/JavaScriptCore/runtime/JSGlobalObject.cpp

index feb22af..3e6ec31 100644 (file)
@@ -1,3 +1,39 @@
+2011-03-18  Geoffrey Garen  <ggaren@apple.com>
+
+        Reviewed by Oliver Hunt.
+
+        [GTK] JSC crashes in 32bit Release bots after r80743
+        https://bugs.webkit.org/show_bug.cgi?id=56180
+        
+        The crash was caused by referencing GC memory from a GC destructor. This
+        is not safe because destruction time / order is not guaranteed.
+
+        * profiler/ProfileGenerator.cpp:
+        (JSC::ProfileGenerator::create):
+        (JSC::ProfileGenerator::ProfileGenerator):
+        (JSC::ProfileGenerator::willExecute):
+        (JSC::ProfileGenerator::didExecute):
+        * profiler/ProfileGenerator.h:
+        (JSC::ProfileGenerator::origin): Made ExecState* the first argument,
+        to match the rest of this class and JSC.
+        
+        Use a JSGlobalObject* instead of an ExecState* with an indirect reference
+        to a JSGlobalObject* to track our origin. This is simpler and more
+        efficient, and it removes the destruction order dependency that was causing
+        our crash.
+
+        * profiler/Profiler.cpp:
+        (JSC::Profiler::startProfiling): Updated for change to JSGlobalObject*.
+        (JSC::Profiler::stopProfiling): New function for stopping all profiles
+        for a given global object. This is more straight-forward than multiplexing
+        through the old function.
+
+        (JSC::dispatchFunctionToProfiles): Updated for change to JSGlobalObject*.
+        * profiler/Profiler.h: Ditto.
+
+        * runtime/JSGlobalObject.cpp:
+        (JSC::JSGlobalObject::~JSGlobalObject): Ditto.
+
 2011-03-17  Geoffrey Garen  <ggaren@apple.com>
 
         Reviewed by Oliver Hunt.
index 68d1733..5db38bc 100644 (file)
@@ -40,19 +40,19 @@ namespace JSC {
 
 static const char* NonJSExecution = "(idle)";
 
-PassRefPtr<ProfileGenerator> ProfileGenerator::create(const UString& title, ExecState* originatingExec, unsigned uid)
+PassRefPtr<ProfileGenerator> ProfileGenerator::create(ExecState* exec, const UString& title, unsigned uid)
 {
-    return adoptRef(new ProfileGenerator(title, originatingExec, uid));
+    return adoptRef(new ProfileGenerator(exec, title, uid));
 }
 
-ProfileGenerator::ProfileGenerator(const UString& title, ExecState* originatingExec, unsigned uid)
-    : m_originatingGlobalExec(originatingExec ? originatingExec->lexicalGlobalObject()->globalExec() : 0)
-    , m_profileGroup(originatingExec ? originatingExec->lexicalGlobalObject()->profileGroup() : 0)
+ProfileGenerator::ProfileGenerator(ExecState* exec, const UString& title, unsigned uid)
+    : m_origin(exec ? exec->lexicalGlobalObject() : 0)
+    , m_profileGroup(exec ? exec->lexicalGlobalObject()->profileGroup() : 0)
 {
     m_profile = Profile::create(title, uid);
     m_currentNode = m_head = m_profile->head();
-    if (originatingExec)
-        addParentForConsoleStart(originatingExec);
+    if (exec)
+        addParentForConsoleStart(exec);
 }
 
 void ProfileGenerator::addParentForConsoleStart(ExecState* exec)
@@ -80,7 +80,7 @@ void ProfileGenerator::willExecute(ExecState* callerCallFrame, const CallIdentif
         JAVASCRIPTCORE_PROFILE_WILL_EXECUTE(m_profileGroup, const_cast<char*>(name.data()), const_cast<char*>(url.data()), callIdentifier.m_lineNumber);
     }
 
-    if (!m_originatingGlobalExec)
+    if (!m_origin)
         return;
 
     ASSERT(m_currentNode);
@@ -95,7 +95,7 @@ void ProfileGenerator::didExecute(ExecState* callerCallFrame, const CallIdentifi
         JAVASCRIPTCORE_PROFILE_DID_EXECUTE(m_profileGroup, const_cast<char*>(name.data()), const_cast<char*>(url.data()), callIdentifier.m_lineNumber);
     }
 
-    if (!m_originatingGlobalExec)
+    if (!m_origin)
         return;
 
     ASSERT(m_currentNode);
index cbed73b..8c8b817 100644 (file)
@@ -34,6 +34,7 @@
 namespace JSC {
 
     class ExecState;
+    class JSGlobalObject;
     class Profile;
     class ProfileNode;
     class UString;
@@ -41,12 +42,12 @@ namespace JSC {
 
     class ProfileGenerator : public RefCounted<ProfileGenerator>  {
     public:
-        static PassRefPtr<ProfileGenerator> create(const UString& title, ExecState* originatingExec, unsigned uid);
+        static PassRefPtr<ProfileGenerator> create(ExecState*, const UString& title, unsigned uid);
 
         // Members
         const UString& title() const;
         PassRefPtr<Profile> profile() const { return m_profile; }
-        ExecState* originatingGlobalExec() const { return m_originatingGlobalExec; }
+        JSGlobalObject* origin() const { return m_origin; }
         unsigned profileGroup() const { return m_profileGroup; }
 
         // Collecting
@@ -61,14 +62,14 @@ namespace JSC {
         typedef void (ProfileGenerator::*ProfileFunction)(ExecState* callerOrHandlerCallFrame, const CallIdentifier& callIdentifier);
 
     private:
-        ProfileGenerator(const UString& title, ExecState* originatingExec, unsigned uid);
+        ProfileGenerator(ExecState*, const UString& title, unsigned uid);
         void addParentForConsoleStart(ExecState*);
 
         void removeProfileStart();
         void removeProfileEnd();
 
         RefPtr<Profile> m_profile;
-        ExecState* m_originatingGlobalExec;
+        JSGlobalObject* m_origin;
         unsigned m_profileGroup;
         RefPtr<ProfileNode> m_head;
         RefPtr<ProfileNode> m_currentNode;
index 301dc0c..bcaaaac 100644 (file)
@@ -66,25 +66,25 @@ void Profiler::startProfiling(ExecState* exec, const UString& title)
 
     // Check if we currently have a Profile for this global ExecState and title.
     // If so return early and don't create a new Profile.
-    ExecState* globalExec = exec ? exec->lexicalGlobalObject()->globalExec() : 0;
+    JSGlobalObject* origin = exec ? exec->lexicalGlobalObject() : 0;
 
     for (size_t i = 0; i < m_currentProfiles.size(); ++i) {
         ProfileGenerator* profileGenerator = m_currentProfiles[i].get();
-        if (profileGenerator->originatingGlobalExec() == globalExec && profileGenerator->title() == title)
+        if (profileGenerator->origin() == origin && profileGenerator->title() == title)
             return;
     }
 
     s_sharedEnabledProfilerReference = this;
-    RefPtr<ProfileGenerator> profileGenerator = ProfileGenerator::create(title, exec, ++ProfilesUID);
+    RefPtr<ProfileGenerator> profileGenerator = ProfileGenerator::create(exec, title, ++ProfilesUID);
     m_currentProfiles.append(profileGenerator);
 }
 
 PassRefPtr<Profile> Profiler::stopProfiling(ExecState* exec, const UString& title)
 {
-    ExecState* globalExec = exec ? exec->lexicalGlobalObject()->globalExec() : 0;
+    JSGlobalObject* origin = exec ? exec->lexicalGlobalObject() : 0;
     for (ptrdiff_t i = m_currentProfiles.size() - 1; i >= 0; --i) {
         ProfileGenerator* profileGenerator = m_currentProfiles[i].get();
-        if (profileGenerator->originatingGlobalExec() == globalExec && (title.isNull() || profileGenerator->title() == title)) {
+        if (profileGenerator->origin() == origin && (title.isNull() || profileGenerator->title() == title)) {
             profileGenerator->stopProfiling();
             RefPtr<Profile> returnProfile = profileGenerator->profile();
 
@@ -99,10 +99,23 @@ PassRefPtr<Profile> Profiler::stopProfiling(ExecState* exec, const UString& titl
     return 0;
 }
 
+void Profiler::stopProfiling(JSGlobalObject* origin)
+{
+    for (ptrdiff_t i = m_currentProfiles.size() - 1; i >= 0; --i) {
+        ProfileGenerator* profileGenerator = m_currentProfiles[i].get();
+        if (profileGenerator->origin() == origin) {
+            profileGenerator->stopProfiling();
+            m_currentProfiles.remove(i);
+            if (!m_currentProfiles.size())
+                s_sharedEnabledProfilerReference = 0;
+        }
+    }
+}
+
 static inline void dispatchFunctionToProfiles(ExecState* callerOrHandlerCallFrame, const Vector<RefPtr<ProfileGenerator> >& profiles, ProfileGenerator::ProfileFunction function, const CallIdentifier& callIdentifier, unsigned currentProfileTargetGroup)
 {
     for (size_t i = 0; i < profiles.size(); ++i) {
-        if (profiles[i]->profileGroup() == currentProfileTargetGroup || !profiles[i]->originatingGlobalExec())
+        if (profiles[i]->profileGroup() == currentProfileTargetGroup || !profiles[i]->origin())
             (profiles[i].get()->*function)(callerOrHandlerCallFrame, callIdentifier);
     }
 }
index f88746d..86366c1 100644 (file)
@@ -38,6 +38,7 @@ namespace JSC {
 
     class ExecState;
     class JSGlobalData;
+    class JSGlobalObject;
     class JSObject;
     class JSValue;
     class ProfileGenerator;
@@ -57,6 +58,7 @@ namespace JSC {
 
         void startProfiling(ExecState*, const UString& title);
         PassRefPtr<Profile> stopProfiling(ExecState*, const UString& title);
+        void stopProfiling(JSGlobalObject*);
 
         void willExecute(ExecState* callerCallFrame, JSValue function);
         void willExecute(ExecState* callerCallFrame, const UString& sourceURL, int startingLineNumber);
index ade054b..89bbe80 100644 (file)
@@ -99,7 +99,7 @@ JSGlobalObject::~JSGlobalObject()
 
     Profiler** profiler = Profiler::enabledProfilerReference();
     if (UNLIKELY(*profiler != 0)) {
-        (*profiler)->stopProfiling(globalExec(), UString());
+        (*profiler)->stopProfiling(this);
     }
 }