fix db thread gc issues by making sure db's clean up outstanding threads when they go away, r/sr=standard8, part of bug 538750
authorDavid Bienvenu <bienvenu@nventure.com>
Fri, 13 Aug 2010 08:27:08 -0700
changeset 6155 6a4f3da12a1e38b421cc917380e98f5b0cf69a6e
parent 6154 31cbbf6d919ee27b9418749544416d6bfe0986d3
child 6156 579f8b02ac2976565b856072b5afa693476187d1
push idunknown
push userunknown
push dateunknown
bugs538750
fix db thread gc issues by making sure db's clean up outstanding threads when they go away, r/sr=standard8, part of bug 538750
mailnews/db/msgdb/public/nsMsgDatabase.h
mailnews/db/msgdb/public/nsMsgThread.h
mailnews/db/msgdb/src/nsMsgDatabase.cpp
mailnews/db/msgdb/src/nsMsgThread.cpp
--- a/mailnews/db/msgdb/public/nsMsgDatabase.h
+++ b/mailnews/db/msgdb/public/nsMsgDatabase.h
@@ -353,17 +353,26 @@ protected:
   nsresult      ClearHdrCache(PRBool reInit);
   nsresult      RemoveHdrFromCache(nsIMsgDBHdr *hdr, nsMsgKey key);
   // all headers currently instantiated, doesn't hold refs
   // these get added when msg hdrs get constructed, and removed when they get destroyed.
   nsresult      GetHdrFromUseCache(nsMsgKey key, nsIMsgDBHdr* *result);
   nsresult      AddHdrToUseCache(nsIMsgDBHdr *hdr, nsMsgKey key); 
   nsresult      ClearUseHdrCache();
   nsresult      RemoveHdrFromUseCache(nsIMsgDBHdr *hdr, nsMsgKey key);
-  
+
+  // not-reference holding array of threads we've handed out.
+  // If a db goes away, it will clean up the outstanding threads.
+  // We use an nsTArray because we don't expect to ever have very many
+  // of these, rarely more than 5.
+  nsTArray<nsMsgThread *> m_threads;
+  // Clear outstanding thread objects
+  void ClearThreads();
+  nsMsgThread *FindExistingThread(nsMsgKey threadId);
+
   mdb_pos       FindInsertIndexInSortedTable(nsIMdbTable *table, mdb_id idToInsert);
 
   void          ClearCachedObjects(PRBool dbGoingAway);
   void          ClearEnumerators();
   // all instantiated headers, but doesn't hold refs. 
   PLDHashTable  *m_headersInUse;
   static PLDHashNumber HashKey(PLDHashTable* aTable, const void* aKey);
   static PRBool MatchEntry(PLDHashTable* aTable, const PLDHashEntryHdr* aEntry, const void* aKey);
--- a/mailnews/db/msgdb/public/nsMsgThread.h
+++ b/mailnews/db/msgdb/public/nsMsgThread.h
@@ -49,28 +49,30 @@ class nsMsgDatabase;
 
 class nsMsgThread : public nsIMsgThread {
 public:
   nsMsgThread();
   nsMsgThread(nsMsgDatabase *db, nsIMdbTable *table);
   virtual ~nsMsgThread();
   
   friend class nsMsgThreadEnumerator;
+  friend class nsMsgDatabase;
   
   NS_DECL_ISUPPORTS
   NS_DECL_NSIMSGTHREAD
     
   // non-interface methods
   nsIMdbTable   *GetMDBTable() {return m_mdbTable;}
   nsIMdbRow		*GetMetaRow() {return m_metaRow;}
   nsMsgDatabase	*m_mdbDB ;
 
 protected:
 
   void                  Init();
+  void                  Clear();
   virtual nsresult      InitCachedValues();
   nsresult              ChangeChildCount(PRInt32 delta);
   nsresult              ChangeUnreadChildCount(PRInt32 delta);
   nsresult              RemoveChild(nsMsgKey msgKey);
   nsresult              SetThreadRootKey(nsMsgKey threadRootKey);
   nsresult              GetChildHdrForKey(nsMsgKey desiredKey, 
                                           nsIMsgDBHdr **result, PRInt32 *resultIndex); 
   nsresult              RerootThread(nsIMsgDBHdr *newParentOfOldRoot, nsIMsgDBHdr *oldRoot, nsIDBChangeAnnouncer *announcer);
--- a/mailnews/db/msgdb/src/nsMsgDatabase.cpp
+++ b/mailnews/db/msgdb/src/nsMsgDatabase.cpp
@@ -560,33 +560,57 @@ void nsMsgDatabase::ClearEnumerators()
   nsTArray<nsMsgDBEnumerator *> copyEnumerators;
   copyEnumerators.SwapElements(m_enumerators);
 
   PRInt32 numEnums = copyEnumerators.Length();
   for (PRUint32 i = 0; i < numEnums; i++)
     copyEnumerators[i]->Clear();
 }
 
+nsMsgThread *nsMsgDatabase::FindExistingThread(nsMsgKey threadId)
+{
+  PRInt32 numThreads = m_threads.Length();
+  for (PRUint32 i = 0; i < numThreads; i++)
+    if (m_threads[i]->m_threadKey == threadId)
+      return m_threads[i];
+
+  return nsnull;
+}
+
+void nsMsgDatabase::ClearThreads()
+{
+  // clear out existing threads
+  nsTArray<nsMsgThread *> copyThreads;
+  copyThreads.SwapElements(m_threads);
+
+  PRInt32 numThreads = copyThreads.Length();
+  for (PRUint32 i = 0; i < numThreads; i++)
+    copyThreads[i]->Clear();
+}
+
 void nsMsgDatabase::ClearCachedObjects(PRBool dbGoingAway)
 {
   ClearHdrCache(PR_FALSE);
 #ifdef DEBUG_DavidBienvenu
   if (m_headersInUse && m_headersInUse->entryCount > 0)
   {
         NS_ASSERTION(PR_FALSE, "leaking headers");
     printf("leaking %d headers in %s\n", m_headersInUse->entryCount, (const char *) m_dbName);
   }
 #endif
+  m_cachedThread = nsnull;
+  m_cachedThreadId = nsMsgKey_None;
   // We should only clear the use hdr cache when the db is going away, or we could
   // end up with multiple copies of the same logical msg hdr, which will lead to
   // ref-counting problems.
   if (dbGoingAway)
+  {
     ClearUseHdrCache();
-  m_cachedThread = nsnull;
-  m_cachedThreadId = nsMsgKey_None;
+    ClearThreads();
+  }
   m_thumb = nsnull;
 }
 
 nsresult nsMsgDatabase::ClearHdrCache(PRBool reInit)
 {
   if (m_cachedHeaders)
   {
     // save this away in case we renter this code.
@@ -3283,18 +3307,23 @@ nsresult nsMsgDBThreadEnumerator::Prefet
     {
       mDone = PR_TRUE;
       return rv;
     }
 
     if (NS_FAILED(rv))
       return rv;
 
-    mResultThread = new nsMsgThread(mDB, table);
-    if(mResultThread)
+    mdbOid tableId;
+    table->GetOid(mDB->GetEnv(), &tableId);
+
+    mResultThread = mDB->FindExistingThread(tableId.mOid_Id);
+    if (!mResultThread)
+      mResultThread = new nsMsgThread(mDB, table);
+    if (mResultThread)
     {
       PRUint32 numChildren = 0;
       NS_ADDREF(mResultThread);
       mResultThread->GetNumChildren(&numChildren);
       // we've got empty thread; don't tell caller about it.
       if (numChildren == 0)
         continue;
     }
@@ -4641,44 +4670,44 @@ nsresult nsMsgDatabase::GetThreadForMsgK
 
   return rv;
 }
 
 // caller needs to unrefer.
 nsIMsgThread *  nsMsgDatabase::GetThreadForThreadId(nsMsgKey threadId)
 {
 
-  if (threadId == m_cachedThreadId && m_cachedThread)
+  nsIMsgThread *retThread = (threadId == m_cachedThreadId && m_cachedThread) ?
+    m_cachedThread : FindExistingThread(threadId);
+  if (retThread)
   {
-    nsIMsgThread *retThread = m_cachedThread;
     NS_ADDREF(retThread);
     return retThread;
   }
-  nsMsgThread *pThread = nsnull;
   if (m_mdbStore)
   {
     mdbOid tableId;
     tableId.mOid_Id = threadId;
     tableId.mOid_Scope = m_hdrRowScopeToken;
 
     nsIMdbTable *threadTable;
     mdb_err res = m_mdbStore->GetTable(GetEnv(), &tableId, &threadTable);
 
     if (NS_SUCCEEDED(res) && threadTable)
     {
-      pThread = new nsMsgThread(this, threadTable);
-      if(pThread)
+      retThread = new nsMsgThread(this, threadTable);
+      if (retThread)
       {
-        NS_ADDREF(pThread);
-        m_cachedThread = pThread;
+        NS_ADDREF(retThread);
+        m_cachedThread = retThread;
         m_cachedThreadId = threadId;
       }
     }
   }
-  return pThread;
+  return retThread;
 }
 
 // make the passed in header a thread header
 nsresult nsMsgDatabase::AddNewThread(nsMsgHdr *msgHdr)
 {
 
   if (!msgHdr)
     return NS_ERROR_NULL_POINTER;
--- a/mailnews/db/msgdb/src/nsMsgThread.cpp
+++ b/mailnews/db/msgdb/src/nsMsgThread.cpp
@@ -39,31 +39,38 @@
 #include "msgCore.h"
 #include "nsMsgThread.h"
 #include "nsMsgDatabase.h"
 #include "nsCOMPtr.h"
 #include "MailNewsTypes2.h"
 
 NS_IMPL_ISUPPORTS1(nsMsgThread, nsIMsgThread)
 
-
 nsMsgThread::nsMsgThread()
 {
   MOZ_COUNT_CTOR(nsMsgThread);
   Init();
 }
 nsMsgThread::nsMsgThread(nsMsgDatabase *db, nsIMdbTable *table)
 {
   MOZ_COUNT_CTOR(nsMsgThread);
   Init();
   m_mdbTable = table;
   m_mdbDB = db;
   if (db)
+  {
     db->AddRef();
-
+    db->m_threads.AppendElement(this);
+  }
+  else
+    NS_ERROR("no db for thread");
+#ifdef DEBUG_David_Bienvenu
+  if (m_mdbDB->m_threads.Length() > 5)
+    printf("more than five outstanding threads\n");
+#endif
   if (table && db)
   {
     table->GetMetaRow(db->GetEnv(), nsnull, nsnull, &m_metaRow);
     InitCachedValues();
   }
 }
 
 void nsMsgThread::Init()
@@ -79,22 +86,31 @@ void nsMsgThread::Init()
   m_newestMsgDate = 0;
   m_cachedValuesInitialized = PR_FALSE;
 }
 
 
 nsMsgThread::~nsMsgThread()
 {
   MOZ_COUNT_DTOR(nsMsgThread);
-  if (m_mdbTable)
-    m_mdbTable->Release();
-  if (m_metaRow)
-    m_metaRow->Release();
   if (m_mdbDB)
-    m_mdbDB->Release();
+  {
+    PRBool found = m_mdbDB->m_threads.RemoveElement(this);
+    NS_ASSERTION(found, "removing thread not in threads array");
+  }
+  else
+    NS_ERROR("null db in thread");
+  Clear();
+}
+
+void nsMsgThread::Clear()
+{
+  NS_IF_RELEASE(m_mdbTable);
+  NS_IF_RELEASE(m_metaRow);
+  NS_IF_RELEASE(m_mdbDB);
 }
 
 nsresult nsMsgThread::InitCachedValues()
 {
   nsresult err = NS_OK;
 
   NS_ENSURE_TRUE(m_mdbDB && m_metaRow, NS_ERROR_INVALID_POINTER);