Regarding Konqueror JavaScript memory leak (bug #44697)

[Dirk Mueller]

On Die, 19 Nov 2002, David Faure wrote:

> In theory all the refcounting due to the KJS wrappers for DOM nodes
> have been cleared already at that point (by m_jscript->clear() above that)
> so I don't believe this is due to JS. Could be wrong there, though. Only
> debugging will tell :)

the scriptinterpreter keeps references to all dom nodes it ever touched, so 
this is leaking heavily and a lot, since dom nodes don't delete their 
children until they're deleted themself. 

below is the patch. it mixes another fix I have in the sources for a few 
months now but never commited (lazy ass I know). it fixes a dangling pointer 
read upon shutdown in the useragent stuff, as during shutdown clear() is 
called but the useragent stuff is already deleted at that point. I move the 
applying of the useragent to initScript() instead. this should work I think 
(never tested it actually), but I've found out that initScript is always 
called upon a new page-view, so it should be safe. 

Please review. I'd really have this in KDE 3.1 because of the severity of 
the leak. but this can also be quite intrusive. luckily we have valgrind now 
to find possible problems quicker :)


-- 
Dirk (received 50 mails today)
-------------- next part --------------
Index: kjs_binding.h
===================================================================
RCS file: /home/kde/kdelibs/khtml/ecma/kjs_binding.h,v
retrieving revision 1.50
diff -u -5 -d -p -r1.50 kjs_binding.h
--- kjs_binding.h	2002/09/26 09:13:47	1.50
+++ kjs_binding.h	2002/11/21 01:06:02
@@ -93,10 +93,13 @@ namespace KJS {
       m_domObjects.insert( objectHandle, obj );
     }
     bool deleteDOMObject( void* objectHandle ) {
       return m_domObjects.remove( objectHandle );
     }
+    void clear() {
+      m_domObjects.clear();
+    }
     /**
      * Static method. Makes all interpreters forget about the object
      */
     static void forgetDOMObject( void* objectHandle );
 
Index: kjs_proxy.cpp
===================================================================
RCS file: /home/kde/kdelibs/khtml/ecma/kjs_proxy.cpp,v
retrieving revision 1.85
diff -u -5 -d -p -r1.85 kjs_proxy.cpp
--- kjs_proxy.cpp	2002/11/12 16:37:46	1.85
+++ kjs_proxy.cpp	2002/11/21 01:06:03
@@ -192,19 +192,20 @@ void KJSProxyImpl::clear() {
     //if (debugWin && debugWin->currentScript() == m_script) {
     //    debugWin->setMode(KJSDebugWin::Stop);
 //        debugWin->leaveSession();
     //}
 #endif
+    m_script->clear( );
+
     Window *win = static_cast<Window *>(m_script->globalObject().imp());
     if (win) {
-        win->clear( m_script->globalExec() );
-        // re-add "debug", clear() removed it
-        m_script->globalObject().put(m_script->globalExec(),
-                                     "debug", Value(new TestFunctionImp()), Internal);
-    }
+      win->clear( m_script->globalExec() );
 
-    applyUserAgent();
+      // re-add "debug", clear() removed it
+      m_script->globalObject().put(m_script->globalExec(),
+                                   "debug", Value(new TestFunctionImp()), Internal);
+    }
   }
 }
 
 DOM::EventListener *KJSProxyImpl::createHTMLEventHandler(QString sourceUrl, QString code)
 {
@@ -295,26 +296,21 @@ void KJSProxyImpl::appendSourceFile(QStr
 #endif
 }
 
 void KJSProxyImpl::initScript()
 {
-  if (m_script)
-    return;
-
-  // Build the global object - which is a Window instance
-  Object globalObject( new Window(m_part) );
+  if ( !m_script ) {
+    // Build the global object - which is a Window instance
+    Object globalObject( new Window(m_part) );
 
-  // Create a KJS interpreter for this part
-  m_script = new KJS::ScriptInterpreter(globalObject, m_part);
-  static_cast<ObjectImp*>(globalObject.imp())->setPrototype(m_script->builtinObjectPrototype());
+    // Create a KJS interpreter for this part
+    m_script = new KJS::ScriptInterpreter(globalObject, m_part);
+    static_cast<ObjectImp*>(globalObject.imp())->setPrototype(m_script->builtinObjectPrototype());
 
-#ifdef KJS_DEBUGGER
-  //m_script->setDebuggingEnabled(m_debugEnabled);
-#endif
-  //m_script->enableDebug();
-  globalObject.put(m_script->globalExec(),
-		   "debug", Value(new TestFunctionImp()), Internal);
+    globalObject.put(m_script->globalExec(),
+    	             "   debug", Value(new TestFunctionImp()), Internal);
+  }
   applyUserAgent();
 }
 
 void KJSProxyImpl::applyUserAgent()
 {
Index: kjs_window.cpp
===================================================================
RCS file: /home/kde/kdelibs/khtml/ecma/kjs_window.cpp,v
retrieving revision 1.298
diff -u -5 -d -p -r1.298 kjs_window.cpp
--- kjs_window.cpp	2002/11/14 11:53:46	1.298
+++ kjs_window.cpp	2002/11/21 01:06:04
@@ -974,11 +974,12 @@ void Window::clear( ExecState *exec )
   delete winq;
   winq = 0L;
   // Get rid of everything, those user vars could hold references to DOM nodes
   deleteAllProperties( exec );
   // Really delete those properties, so that the DOM nodes get deref'ed
-  KJS::Collector::collect();
+  while(KJS::Collector::collect())
+      ;
   if (!m_part.isNull()) {
     KJSProxy* proxy = KJSProxy::proxy( m_part );
     if (proxy) // i.e. JS not disabled
     {
       winq = new WindowQObject(this);