Changeset 47405 in webkit for trunk/JavaScriptCore

Timestamp:

Aug 17, 2009, 6:05:37 PM (16 years ago)

Author:

[email protected]

Message:

Fix 300,000+ leaks seen during the regression tests.

Reviewed by Darin Adler.

EvalCodeCache::get was heap-allocating an EvalExecutable instance without adopting the initial reference.
While fixing this we noticed that EvalExecutable was a RefCounted type that was sometimes stack allocated.
To make this cleaner and to prevent clients from attempting to ref a stack-allocated instance, we move the
refcounting down to a new CacheableEvalExecutable class that derives from EvalExecutable. EvalCodeCache::get
now uses CacheableEvalExecutable::create and avoids the leak.

• bytecode/EvalCodeCache.h:

(JSC::EvalCodeCache::get):

• interpreter/Interpreter.cpp:

(JSC::Interpreter::callEval):

• runtime/Executable.h:

(JSC::CacheableEvalExecutable::create):
(JSC::CacheableEvalExecutable::CacheableEvalExecutable):

Location:

trunk/JavaScriptCore

Files:

• ChangeLog (modified) (1 diff)
• bytecode/EvalCodeCache.h (modified) (3 diffs)
• interpreter/Interpreter.cpp (modified) (1 diff)
• runtime/Executable.h (modified) (2 diffs)

trunk/JavaScriptCore/ChangeLog

@@ +1 @@
+2009-08-17  Mark Rowe  <[email protected]>
+
+        Reviewed by Darin Adler.
+
+        Fix 300,000+ leaks seen during the regression tests.
+
+        EvalCodeCache::get was heap-allocating an EvalExecutable instance without adopting the initial reference.
+        While fixing this we noticed that EvalExecutable was a RefCounted type that was sometimes stack allocated.
+        To make this cleaner and to prevent clients from attempting to ref a stack-allocated instance, we move the
+        refcounting down to a new CacheableEvalExecutable class that derives from EvalExecutable. EvalCodeCache::get
+        now uses CacheableEvalExecutable::create and avoids the leak.
+
+        * bytecode/EvalCodeCache.h:
+        (JSC::EvalCodeCache::get):
+        * interpreter/Interpreter.cpp:
+        (JSC::Interpreter::callEval):
+        * runtime/Executable.h:
+        (JSC::CacheableEvalExecutable::create):
+        (JSC::CacheableEvalExecutable::CacheableEvalExecutable):
+
 2009-08-17  Oliver Hunt  <[email protected]>
 

trunk/JavaScriptCore/bytecode/EvalCodeCache.h

@@ -43 +43 @@
     class EvalCodeCache {
     public:
-        PassRefPtr<EvalExecutable> get(ExecState* exec, const UString& evalSource, ScopeChainNode* scopeChain, JSValue& exceptionValue)
+        PassRefPtr<CacheableEvalExecutable> get(ExecState* exec, const UString& evalSource, ScopeChainNode* scopeChain, JSValue& exceptionValue)
         {
-            RefPtr<EvalExecutable> evalExecutable;
+            RefPtr<CacheableEvalExecutable> evalExecutable;
 
             if (evalSource.size() < maxCacheableSourceLength && (*scopeChain->begin())->isVariableObject())
@@ -51 +51 @@
 
             if (!evalExecutable) {
-                evalExecutable = new EvalExecutable(makeSource(evalSource));
+                evalExecutable = CacheableEvalExecutable::create(makeSource(evalSource));
                 exceptionValue = evalExecutable->parse(exec);
                 if (exceptionValue)
@@ -75 +75 @@
         static const int maxCacheEntries = 64;
 
-        typedef HashMap<RefPtr<UString::Rep>, RefPtr<EvalExecutable> > EvalCacheMap;
+        typedef HashMap<RefPtr<UString::Rep>, RefPtr<CacheableEvalExecutable> > EvalCacheMap;
         EvalCacheMap m_cacheMap;
     };

trunk/JavaScriptCore/interpreter/Interpreter.cpp

@@ -351 +351 @@
     if (JSValue parsedObject = preparser.tryLiteralParse())
         return parsedObject;
-    
-    
+
     ScopeChainNode* scopeChain = callFrame->scopeChain();
     CodeBlock* codeBlock = callFrame->codeBlock();
-    RefPtr<EvalExecutable> eval = codeBlock->evalCodeCache().get(callFrame, programSource, scopeChain, exceptionValue);
+    RefPtr<CacheableEvalExecutable> eval = codeBlock->evalCodeCache().get(callFrame, programSource, scopeChain, exceptionValue);
 
     JSValue result = jsUndefined();

trunk/JavaScriptCore/runtime/Executable.h

@@ -57 +57 @@
     };
 
-    class EvalExecutable : public TemplateExecutable<EvalNode, EvalCodeBlock>, public RefCounted<EvalExecutable> {
+    class EvalExecutable : public TemplateExecutable<EvalNode, EvalCodeBlock> {
     public:
         EvalExecutable(const SourceCode& source)
@@ -65 +65 @@
 
         JSObject* parse(ExecState* exec, bool allowDebug = true);
+    };
+
+    class CacheableEvalExecutable : public EvalExecutable, public RefCounted<CacheableEvalExecutable> {
+    public:
+        static PassRefPtr<CacheableEvalExecutable> create(const SourceCode& source) { return adoptRef(new CacheableEvalExecutable(source)); }
+
+    private:
+        CacheableEvalExecutable(const SourceCode& source)
+            : EvalExecutable(source)
+        {
+        }
     };