Bug 16105. (Bugsplat 366256.) Ensure that nsISupports::Release() calls dtor only once by 'stabilizing' refcnt to a non-zero value before invoking the dtor. See also news://news.mozilla.org/37FD0F3C.3078AE5C%40netscape.com. r=scc,fur

content/xul/content/src/nsXULElement.cpp

@@ -694,7 +694,6 @@ RDFElementImpl::~RDFElementImpl()
     NS_IF_RELEASE(mListenerManager);
 
     if (mChildren) {
-#if 0
         // Force child's parent to be null. This ensures that we don't
         // have dangling pointers if a child gets leaked.
         PRUint32 cnt;
@@ -706,7 +705,6 @@ RDFElementImpl::~RDFElementImpl()
 
             child->SetParent(nsnull);
         }
-#endif
         NS_RELEASE(mChildren);
     }
 

docshell/base/nsWebShell.cpp

@@ -4210,7 +4210,6 @@ nsWebShellFactory::nsWebShellFactory()
 
 nsWebShellFactory::~nsWebShellFactory()
 {
-  NS_ASSERTION(mRefCnt == 0, "non-zero refcnt at destruction");
 }
 
 nsresult

netwerk/mime/src/nsMIMEServiceFactory.cpp

@@ -334,7 +334,6 @@ nsMIMEServiceFactory::nsMIMEServiceFactory(const nsCID &aClass,
 
 nsMIMEServiceFactory::~nsMIMEServiceFactory()
 {
-    NS_ASSERTION(mRefCnt == 0, "non-zero refcnt at destruction");
 }
 
 NS_IMETHODIMP

netwerk/protocol/http/src/nsHTTPHandlerFactory.cpp

@@ -50,7 +50,6 @@ nsHTTPHandlerFactory::nsHTTPHandlerFactory(const nsCID &aClass)
 
 nsHTTPHandlerFactory::~nsHTTPHandlerFactory()
 {
-    NS_ASSERTION(mRefCnt == 0, "non-zero refcnt at destruction");
 }
 
 NS_IMETHODIMP

netwerk/streamconv/converters/nsFTPDirListingConv.cpp

@@ -1017,7 +1017,6 @@ FTPDirListingFactory::FTPDirListingFactory(const nsCID &aClass,
 
 FTPDirListingFactory::~FTPDirListingFactory()
 {
-    NS_ASSERTION(mRefCnt == 0, "non-zero refcnt at destruction");
 }
 
 NS_IMPL_ISUPPORTS(FTPDirListingFactory, NS_GET_IID(nsIFactory));

netwerk/streamconv/converters/nsMultiMixedConv.cpp

@@ -383,7 +383,6 @@ MultiMixedFactory::MultiMixedFactory(const nsCID &aClass,
 
 MultiMixedFactory::~MultiMixedFactory()
 {
-    NS_ASSERTION(mRefCnt == 0, "non-zero refcnt at destruction");
 }
 
 NS_IMPL_ISUPPORTS(MultiMixedFactory, NS_GET_IID(nsIFactory));

netwerk/streamconv/test/Converters.cpp

@@ -116,7 +116,6 @@ TestConverterFactory::TestConverterFactory(const nsCID &aClass,
 
 TestConverterFactory::~TestConverterFactory()
 {
-    NS_ASSERTION(mRefCnt == 0, "non-zero refcnt at destruction");
 }
 
 NS_IMPL_ISUPPORTS(TestConverterFactory, NS_GET_IID(nsIFactory));

netwerk/test/TestStreamConv.cpp

@@ -132,7 +132,6 @@ TestConverterFactory::TestConverterFactory(const nsCID &aClass,
 
 TestConverterFactory::~TestConverterFactory()
 {
-    NS_ASSERTION(mRefCnt == 0, "non-zero refcnt at destruction");
 }
 
 NS_IMETHODIMP

network/module/nsNetFactory.cpp

@@ -73,7 +73,6 @@ nsNetFactory::nsNetFactory(const nsCID &aClass)
 
 nsNetFactory::~nsNetFactory()   
 {   
-  NS_ASSERTION(mRefCnt == 0, "non-zero refcnt at destruction");   
 }   
 
 nsresult nsNetFactory::QueryInterface(const nsIID &aIID,   

profile/Acctidl/nsAccountServicesFactory.cpp

@@ -71,7 +71,6 @@ AccountServicesFactoryImpl::AccountServicesFactoryImpl(const nsCID &aClass,
 
 AccountServicesFactoryImpl::~AccountServicesFactoryImpl()
 {
-    NS_ASSERTION(mRefCnt == 0, "non-zero refcnt at destruction");
 }
 
 NS_IMETHODIMP

rdf/content/src/nsRDFElement.cpp

@@ -694,7 +694,6 @@ RDFElementImpl::~RDFElementImpl()
     NS_IF_RELEASE(mListenerManager);
 
     if (mChildren) {
-#if 0
         // Force child's parent to be null. This ensures that we don't
         // have dangling pointers if a child gets leaked.
         PRUint32 cnt;
@@ -706,7 +705,6 @@ RDFElementImpl::~RDFElementImpl()
 
             child->SetParent(nsnull);
         }
-#endif
         NS_RELEASE(mChildren);
     }
 

rdf/content/src/nsXULElement.cpp

@@ -694,7 +694,6 @@ RDFElementImpl::~RDFElementImpl()
     NS_IF_RELEASE(mListenerManager);
 
     if (mChildren) {
-#if 0
         // Force child's parent to be null. This ensures that we don't
         // have dangling pointers if a child gets leaked.
         PRUint32 cnt;
@@ -706,7 +705,6 @@ RDFElementImpl::~RDFElementImpl()
 
             child->SetParent(nsnull);
         }
-#endif
         NS_RELEASE(mChildren);
     }
 

view/src/nsViewFactory.cpp

@@ -63,7 +63,6 @@ nsViewFactory::nsViewFactory(const nsCID &aClass)
 
 nsViewFactory::~nsViewFactory()   
 {   
-	NS_ASSERTION(mRefCnt == 0, "non-zero refcnt at destruction");   
 }   
 
 NS_IMPL_ISUPPORTS1(nsViewFactory, nsIFactory)

webshell/src/nsWebShell.cpp

@@ -4210,7 +4210,6 @@ nsWebShellFactory::nsWebShellFactory()
 
 nsWebShellFactory::~nsWebShellFactory()
 {
-  NS_ASSERTION(mRefCnt == 0, "non-zero refcnt at destruction");
 }
 
 nsresult

webshell/tests/viewer/nsBrowserWindow.cpp

@@ -3424,7 +3424,6 @@ nsBrowserWindowFactory::nsBrowserWindowFactory()
 
 nsBrowserWindowFactory::~nsBrowserWindowFactory()
 {
-  NS_ASSERTION(mRefCnt == 0, "non-zero refcnt at destruction");
 }
 
 nsresult

webshell/tests/viewer/nsXPBaseWindow.cpp

@@ -782,7 +782,6 @@ nsXPBaseWindowFactory::nsXPBaseWindowFactory()
 //----------------------------------------------------------------------
 nsXPBaseWindowFactory::~nsXPBaseWindowFactory()
 {
-  NS_ASSERTION(mRefCnt == 0, "non-zero refcnt at destruction");
 }
 
 //----------------------------------------------------------------------

widget/src/beos/nsWidgetFactory.cpp

@@ -121,7 +121,6 @@ nsWidgetFactory::nsWidgetFactory(const nsCID &aClass)
 
 nsWidgetFactory::~nsWidgetFactory()   
 {   
-  NS_ASSERTION(mRefCnt == 0, "Reference count not zero in destructor");
 }   
 
 nsresult nsWidgetFactory::QueryInterface(const nsIID &aIID,   

widget/src/build/nsWinWidgetFactory.cpp

@@ -243,9 +243,6 @@ nsWidgetFactory::nsWidgetFactory(const nsCID &aClass)
 nsWidgetFactory::~nsWidgetFactory()   
 
 {   
-
-    NS_ASSERTION(mRefCnt == 0, "Reference count not zero in destructor");   
-
 }   
 
 

widget/src/gtk/nsWidgetFactory.cpp

@@ -125,7 +125,6 @@ nsWidgetFactory::nsWidgetFactory(const nsCID &aClass)
 
 nsWidgetFactory::~nsWidgetFactory()   
 {   
-  NS_ASSERTION(mRefCnt == 0, "Reference count not zero in destructor");
 }   
 
 nsresult nsWidgetFactory::QueryInterface(const nsIID &aIID,   

widget/src/motif/app_context/nsMotifAppContextServiceFactory.cpp

@@ -53,7 +53,6 @@ nsMotifAppContextServiceFactory::nsMotifAppContextServiceFactory(const nsCID &aC
 
 nsMotifAppContextServiceFactory::~nsMotifAppContextServiceFactory()
 {
-  NS_ASSERTION(mRefCnt == 0, "non-zero refcnt at destruction");
 }
 
 NS_IMPL_ISUPPORTS(nsMotifAppContextServiceFactory, nsIFactory::GetIID())

widget/src/motif/nsWidgetFactory.cpp

@@ -123,7 +123,6 @@ nsWidgetFactory::nsWidgetFactory(const nsCID &aClass)
 
 nsWidgetFactory::~nsWidgetFactory()   
 {   
-  NS_ASSERTION(mRefCnt == 0, "Reference count not zero in destructor");
 }   
 
 nsresult nsWidgetFactory::QueryInterface(const nsIID &aIID,   

widget/src/photon/nsWidgetFactory.cpp

@@ -135,8 +135,6 @@ nsWidgetFactory::nsWidgetFactory(const nsCID &aClass)
 nsWidgetFactory::~nsWidgetFactory()   
 {   
   PR_LOG(PhWidLog, PR_LOG_DEBUG,("nsWidgetFactory::~nsWidgetFactory Destructor Called\n"));
-
-  NS_ASSERTION(mRefCnt == 0, "Reference count not zero in destructor");
 }   
 
 nsresult nsWidgetFactory::QueryInterface(const nsIID &aIID,   

widget/src/qt/nsWidgetFactory.cpp

@@ -127,7 +127,6 @@ nsWidgetFactory::nsWidgetFactory(const nsCID &aClass)
 
 nsWidgetFactory::~nsWidgetFactory()   
 {   
-    NS_ASSERTION(mRefCnt == 0, "Reference count not zero in destructor");
 }   
 
 nsresult nsWidgetFactory::QueryInterface(const nsIID &aIID,   

widget/src/unix_services/toolkit_service/nsUnixToolkitServiceFactory.cpp

@@ -53,7 +53,6 @@ nsUnixToolkitServiceFactory::nsUnixToolkitServiceFactory(const nsCID &aClass) :
 
 nsUnixToolkitServiceFactory::~nsUnixToolkitServiceFactory()
 {
-  NS_ASSERTION(mRefCnt == 0, "non-zero refcnt at destruction");
 }
 
 NS_IMPL_ISUPPORTS(nsUnixToolkitServiceFactory, nsIFactory::GetIID())

widget/src/xlib/nsWidgetFactory.cpp

@@ -100,7 +100,6 @@ nsWidgetFactory::nsWidgetFactory(const nsCID &aClass)
 
 nsWidgetFactory::~nsWidgetFactory()
 {
-  NS_ASSERTION(mRefCnt == 0, "Reference count not zero in destructor");   
 }
 
 nsresult nsWidgetFactory::QueryInterface(const nsIID &aIID,

widget/src/xlib/window_service/nsXlibWindowServiceFactory.cpp

@@ -54,7 +54,6 @@ nsXlibWindowServiceFactory::nsXlibWindowServiceFactory(const nsCID &aClass) :
 
 nsXlibWindowServiceFactory::~nsXlibWindowServiceFactory()
 {
-  NS_ASSERTION(mRefCnt == 0, "non-zero refcnt at destruction");
 }
 
 NS_IMPL_ISUPPORTS(nsXlibWindowServiceFactory, nsIFactory::GetIID())

widget/timer/src/unix/gtk/nsTimerGtkFactory.cpp

@@ -56,7 +56,6 @@ nsTimerGtkFactory::nsTimerGtkFactory(const nsCID &aClass) :
 
 nsTimerGtkFactory::~nsTimerGtkFactory()   
 {   
-  NS_ASSERTION(mRefCnt == 0, "non-zero refcnt at destruction");   
 }   
 
 NS_IMPL_ISUPPORTS(nsTimerGtkFactory, nsIFactory::GetIID())

widget/timer/src/unix/motif/nsTimerMotifFactory.cpp

@@ -56,7 +56,6 @@ nsTimerMotifFactory::nsTimerMotifFactory(const nsCID &aClass) :
 
 nsTimerMotifFactory::~nsTimerMotifFactory()   
 {   
-  NS_ASSERTION(mRefCnt == 0, "non-zero refcnt at destruction");   
 }   
 
 NS_IMPL_ISUPPORTS(nsTimerMotifFactory, nsIFactory::GetIID())

widget/timer/src/unix/qt/nsTimerQtFactory.cpp

@@ -56,7 +56,6 @@ nsTimerQtFactory::nsTimerQtFactory(const nsCID &aClass) :
 
 nsTimerQtFactory::~nsTimerQtFactory()   
 {   
-  NS_ASSERTION(mRefCnt == 0, "non-zero refcnt at destruction");   
 }   
 
 NS_IMPL_ISUPPORTS(nsTimerQtFactory, nsIFactory::GetIID())

widget/timer/src/unix/xlib/nsTimerXlibFactory.cpp

@@ -56,7 +56,6 @@ nsTimerXlibFactory::nsTimerXlibFactory(const nsCID &aClass) :
 
 nsTimerXlibFactory::~nsTimerXlibFactory()   
 {   
-  NS_ASSERTION(mRefCnt == 0, "non-zero refcnt at destruction");   
 }   
 
 NS_IMPL_ISUPPORTS(nsTimerXlibFactory, nsIFactory::GetIID())

xpcom/base/nsAgg.h

@@ -112,6 +112,7 @@ _class::Internal::Release(void)                                             \
     --agg->mRefCnt;                                                         \
     NS_LOG_RELEASE(this, agg->mRefCnt, #_class);                            \
     if (agg->mRefCnt == 0) {                                                \
+        agg->mRefCnt = 1; /* stabilize */                                   \
         NS_DELETEXPCOM(agg);                                                \
         return 0;                                                           \
     }                                                                       \

xpcom/base/nsISupportsUtils.h

@@ -200,6 +200,15 @@ NS_IMETHODIMP_(nsrefcnt) _class::AddRef(void)                \
 /**
  * Use this macro to implement the Release method for a given <i>_class</i>
  * @param _class The name of the class implementing the method
+ *
+ * A note on the 'stabilization' of the refcnt to one. At that point,
+ * the object's refcount will have gone to zero. The object's
+ * destructor may trigger code that attempts to QueryInterface() and
+ * Release() 'this' again. Doing so will temporarily increment and
+ * decrement the refcount. (Only a logic error would make one try to
+ * keep a permanent hold on 'this'.)  To prevent re-entering the
+ * destructor, we make sure that no balanced refcounting can return
+ * the refcount to |0|.
  */
 #define NS_IMPL_RELEASE(_class)                              \
 NS_IMETHODIMP_(nsrefcnt) _class::Release(void)               \
@@ -208,6 +217,7 @@ NS_IMETHODIMP_(nsrefcnt) _class::Release(void)               \
   --mRefCnt;                                                 \
   NS_LOG_RELEASE(this, mRefCnt, #_class);                    \
   if (mRefCnt == 0) {                                        \
+    mRefCnt = 1; /* stabilize */                             \
     NS_DELETEXPCOM(this);                                    \
     return 0;                                                \
   }                                                          \

xpcom/components/nsXPComFactory.h

@@ -110,10 +110,7 @@ public:                                                           \
   }                                                               \
                                                                   \
 protected:                                                        \
-  virtual ~ns##_name##Factory()                                   \
-  {                                                               \
-    NS_ASSERTION(mRefCnt == 0, "non-zero refcnt at destruction"); \
-  }                                                               \
+  virtual ~ns##_name##Factory() {}                                \
 };                                                                \
 NS_IMPL_ISUPPORTS1(ns##_name##Factory, nsIFactory);
 

xpcom/glue/nsISupportsUtils.h

@@ -200,6 +200,15 @@ NS_IMETHODIMP_(nsrefcnt) _class::AddRef(void)                \
 /**
  * Use this macro to implement the Release method for a given <i>_class</i>
  * @param _class The name of the class implementing the method
+ *
+ * A note on the 'stabilization' of the refcnt to one. At that point,
+ * the object's refcount will have gone to zero. The object's
+ * destructor may trigger code that attempts to QueryInterface() and
+ * Release() 'this' again. Doing so will temporarily increment and
+ * decrement the refcount. (Only a logic error would make one try to
+ * keep a permanent hold on 'this'.)  To prevent re-entering the
+ * destructor, we make sure that no balanced refcounting can return
+ * the refcount to |0|.
  */
 #define NS_IMPL_RELEASE(_class)                              \
 NS_IMETHODIMP_(nsrefcnt) _class::Release(void)               \
@@ -208,6 +217,7 @@ NS_IMETHODIMP_(nsrefcnt) _class::Release(void)               \
   --mRefCnt;                                                 \
   NS_LOG_RELEASE(this, mRefCnt, #_class);                    \
   if (mRefCnt == 0) {                                        \
+    mRefCnt = 1; /* stabilize */                             \
     NS_DELETEXPCOM(this);                                    \
     return 0;                                                \
   }                                                          \

xpcom/sample/nsSampleFactory.cpp

@@ -119,7 +119,6 @@ SampleFactoryImpl::SampleFactoryImpl(const nsCID &aClass,
 
 SampleFactoryImpl::~SampleFactoryImpl()
 {
-    NS_ASSERTION(mRefCnt == 0, "non-zero refcnt at destruction");
 }
 
 /**

xpfe/apprunner/src/nsAppRunnerImpl.cpp

@@ -325,7 +325,6 @@ nsAppRunnerFactory::nsAppRunnerFactory() {
 }
 
 nsAppRunnerFactory::~nsAppRunnerFactory() {
-    NS_ASSERTION(mRefCnt == 0, "non-zero refcnt at destruction");
 }
 
 NS_IMPL_ISUPPORTS(nsAppRunnerFactory, kIFactoryIID);

xpfe/appshell/src/nsCommandLineService.cpp

@@ -318,7 +318,6 @@ nsCmdLineServiceFactory::LockFactory(PRBool lock)
 
 nsCmdLineServiceFactory::~nsCmdLineServiceFactory()
 {
-  NS_ASSERTION(mRefCnt == 0, "non-zero refcnt at destruction");
 }
 
 NS_IMPL_ISUPPORTS(nsCmdLineServiceFactory, kIFactoryIID);

xpfe/appshell/src/nsFileLocations.cpp

@@ -628,7 +628,6 @@ nsresult nsFileLocatorFactory::LockFactory(PRBool /*lock*/)
 nsFileLocatorFactory::~nsFileLocatorFactory()
 //----------------------------------------------------------------------------------------
 {
-  NS_ASSERTION(mRefCnt == 0, "non-zero refcnt at destruction");
 }
 
 NS_IMPL_ISUPPORTS(nsFileLocatorFactory, kIFactoryIID);

xpfe/appshell/src/nsNetSupportDialog.cpp

@@ -841,7 +841,6 @@ nsNetSupportDialogFactory::LockFactory(PRBool lock)
 
 nsNetSupportDialogFactory::~nsNetSupportDialogFactory()
 {
-  NS_ASSERTION(mRefCnt == 0, "non-zero refcnt at destruction");
 }
 
 NS_IMPL_ISUPPORTS(nsNetSupportDialogFactory, kIFactoryIID);

xpfe/appshell/src/nsSessionHistory.cpp

@@ -1508,7 +1508,6 @@ nsSessionHistoryFactory::LockFactory(PRBool aLock)
 
 nsSessionHistoryFactory::~nsSessionHistoryFactory()
 {
-  NS_ASSERTION(mRefCnt == 0, "non-zero refcnt at destruction");
 }
 
 NS_IMPL_ISUPPORTS(nsSessionHistoryFactory, kIFactoryIID);

xpfe/components/prefwindow/src/nsPrefWindowFactory.cpp

@@ -73,7 +73,6 @@ nsPrefWindowFactory::nsPrefWindowFactory(
 nsPrefWindowFactory::~nsPrefWindowFactory()   
 //----------------------------------------------------------------------------------------
 {
-	NS_ASSERTION(mRefCnt == 0, "non-zero refcnt at destruction");   
 //	nsCRT::free(mClassName);
 //	nsCRT::free(mProgID);
 }   

xpfe/components/startup/src/nsCommandLineService.cpp

@@ -318,7 +318,6 @@ nsCmdLineServiceFactory::LockFactory(PRBool lock)
 
 nsCmdLineServiceFactory::~nsCmdLineServiceFactory()
 {
-  NS_ASSERTION(mRefCnt == 0, "non-zero refcnt at destruction");
 }
 
 NS_IMPL_ISUPPORTS(nsCmdLineServiceFactory, kIFactoryIID);