After more than 30 minutes inactivity for an imap connection, selecting a message crashes the system. Steps to reproduce: 1) add an imap account using the following prefs entries, make appropriate change as you wish. user_pref("mail.accountmanager.accounts", "qatest21,..."); user_pref("mail.account.qatest21.identities", "qatest21-netscape"); user_pref("mail.account.qatest21.server", "dredd"); user_pref("mail.identity.qatest21-netscape.fullName", "QA test 21"); user_pref("mail.identity.qatest21-netscape.send_html", false); user_pref("mail.identity.qatest21-netscape.smtp_name", "qatest21"); user_pref("mail.identity.qatest21-netscape.smtp_server", "dredd.mcom.com"); user_pref("mail.identity.qatest21-netscape.useremail", "qatest21@netscape.com"); user_pref("mail.server.dredd.check_new_mail", false); user_pref("mail.server.dredd.directory", "c:\\Program Files\\Netscape\\Users\\qatest21\\imapMail\\dredd"); user_pref("mail.server.dredd.hostname", "dredd"); user_pref("mail.server.dredd.max_cached_connections", 3); user_pref("mail.server.dredd.name", "Dredd the almighty"); user_pref("mail.server.dredd.password", "Ne!sc-pe"); user_pref("mail.server.dredd.timeout", 29); user_pref("mail.server.dredd.type", "imap"); user_pref("mail.server.dredd.userName", "qatest21"); 2) launch "appruner -mail" with mail enabled 3) double click on dredd imap server 4) select Inbox 5) let it sit for more than 30 minutes 6) select a message to display after more that 30 minutes passed 7) you should be crash for now Fix in hand. Tested and ready to check in. ? imapTimeoutPatch.txt ? makelog ? updatelog ? tests/harness/Debug ? tests/harness/makefile1.dsp ? tests/harness/makefile1.dsw ? tests/harness/makefile1.ncb ? tests/harness/makefile1.opt ? tests/harness/makefile1.plg Index: public/nsIImapIncomingServer.h =================================================================== RCS file: /cvsroot/mozilla/mailnews/imap/public/nsIImapIncomingServer.h,v retrieving revision 1.4 diff -c -r1.4 nsIImapIncomingServer.h *** nsIImapIncomingServer.h 1999/05/18 22:32:46 1.4 --- nsIImapIncomingServer.h 1999/05/20 13:47:49 *************** *** 49,54 **** --- 49,57 ---- // ** attribute maximum cached connections ** NS_IMETHOD GetMaximumConnectionsNumber(PRInt32* maxConnections) = 0; NS_IMETHOD SetMaximumConnectionsNumber(PRInt32 maxConnections) = 0; + + NS_IMETHOD GetTimeOutLimits(PRInt32* minutes) = 0; + NS_IMETHOD SetTimeOutLimits(PRInt32 minutes) = 0; NS_IMETHOD GetImapConnectionAndLoadUrl(nsIEventQueue* aClientEventQueue, nsIImapUrl* aImapUrl, Index: public/nsIImapProtocol.h =================================================================== RCS file: /cvsroot/mozilla/mailnews/imap/public/nsIImapProtocol.h,v retrieving revision 1.15 diff -c -r1.15 nsIImapProtocol.h *** nsIImapProtocol.h 1999/05/18 22:33:49 1.15 --- nsIImapProtocol.h 1999/05/20 13:47:49 *************** *** 24,29 **** --- 24,30 ---- #include "nsIStreamListener.h" #include "nsIOutputStream.h" #include "plevent.h" + #include "prtime.h" /* include all of our event sink interfaces */ *************** *** 97,102 **** --- 98,105 ---- // Tell thread to die NS_IMETHOD TellThreadToDie(PRBool isSafeToDie) = 0; + // Get last active time stamp + NS_IMETHOD GetLastActiveTimeStamp(PRTime *aTimeStamp) = 0; }; #endif /* nsIImapProtocol_h___ */ Index: src/nsImapIncomingServer.cpp =================================================================== RCS file: /cvsroot/mozilla/mailnews/imap/src/nsImapIncomingServer.cpp,v retrieving revision 1.8 diff -c -r1.8 nsImapIncomingServer.cpp *** nsImapIncomingServer.cpp 1999/05/18 23:24:07 1.8 --- nsImapIncomingServer.cpp 1999/05/20 13:47:49 *************** *** 58,63 **** --- 58,66 ---- // we support the nsIImapIncomingServer interface NS_IMETHOD GetMaximumConnectionsNumber(PRInt32* maxConnections); NS_IMETHOD SetMaximumConnectionsNumber(PRInt32 maxConnections); + + NS_IMETHOD GetTimeOutLimits(PRInt32* minutes); + NS_IMETHOD SetTimeOutLimits(PRInt32 minutes); NS_IMETHOD GetImapConnectionAndLoadUrl(nsIEventQueue* aClientEventQueue, nsIImapUrl* aImapUrl, *************** *** 71,76 **** --- 74,80 ---- nsresult CreateImapConnection (nsIEventQueue* aEventQueue, nsIImapUrl* aImapUrl, nsIImapProtocol** aImapConnection); + PRBool ConnectionTimeOut(nsIImapProtocol* aImapConnection); char *m_rootFolderPath; nsCOMPtr<nsISupportsArray> m_connectionCache; nsCOMPtr<nsISupportsArray> m_urlQueue; *************** *** 144,149 **** --- 148,156 ---- NS_IMPL_SERVERPREF_INT(nsImapIncomingServer, MaximumConnectionsNumber, "max_cached_connections"); + NS_IMPL_SERVERPREF_INT(nsImapIncomingServer, TimeOutLimits, + "timeout"); + NS_IMETHODIMP nsImapIncomingServer::GetImapConnectionAndLoadUrl(nsIEventQueue* aClientEventQueue, *************** *** 194,201 **** m_urlQueue->Count(&cnt); if (cnt > 0) { nsCOMPtr<nsIImapUrl> ! aImapUrl(do_QueryInterface(m_urlQueue->ElementAt(0))); if (aImapUrl) { --- 201,210 ---- m_urlQueue->Count(&cnt); if (cnt > 0) { + nsCOMPtr<nsISupports> + aSupport(getter_AddRefs(m_urlQueue->ElementAt(0))); nsCOMPtr<nsIImapUrl> ! aImapUrl(do_QueryInterface(aSupport, &rv)); if (aImapUrl) { *************** *** 225,248 **** NS_IMETHODIMP nsImapIncomingServer::RemoveConnection(nsIImapProtocol* aImapConnection) { - PRInt32 elementIndex = -1; - nsresult rv; PR_CEnterMonitor(this); if (aImapConnection) - { - // preventing earlier release of the protocol - nsCOMPtr<nsIImapProtocol> - aConnection(do_QueryInterface(aImapConnection,&rv)); - aImapConnection->TellThreadToDie(PR_TRUE); - m_connectionCache->RemoveElement(aImapConnection); - } PR_CExitMonitor(this); return NS_OK; } nsresult nsImapIncomingServer::CreateImapConnection(nsIEventQueue *aEventQueue, nsIImapUrl * aImapUrl, --- 234,290 ---- NS_IMETHODIMP nsImapIncomingServer::RemoveConnection(nsIImapProtocol* aImapConnection) { PR_CEnterMonitor(this); if (aImapConnection) m_connectionCache->RemoveElement(aImapConnection); PR_CExitMonitor(this); return NS_OK; } + PRBool + nsImapIncomingServer::ConnectionTimeOut(nsIImapProtocol* aConnection) + { + PRBool retVal = PR_FALSE; + if (!aConnection) return retVal; + nsresult rv; + + PR_CEnterMonitor(this); + PRInt32 timeoutInMinutes = 0; + rv = GetTimeOutLimits(&timeoutInMinutes); + if (NS_FAILED(rv) || timeoutInMinutes <= 0 || timeoutInMinutes > 29) + { + timeoutInMinutes = 29; + SetTimeOutLimits(timeoutInMinutes); + } + + PRTime cacheTimeoutLimits; + + LL_I2L(cacheTimeoutLimits, timeoutInMinutes * 60 * 1000000); // in + // microseconds + PRTime lastActiveTimeStamp; + rv = aConnection->GetLastActiveTimeStamp(&lastActiveTimeStamp); + + PRTime elapsedTime; + LL_SUB(elapsedTime, PR_Now(), lastActiveTimeStamp); + PRTime t; + LL_SUB(t, elapsedTime, cacheTimeoutLimits); + if (LL_GE_ZERO(t)) + { + nsCOMPtr<nsIImapProtocol> aProtocol(do_QueryInterface(aConnection, + &rv)); + if (NS_SUCCEEDED(rv) && aProtocol) + { + m_connectionCache->RemoveElement(aConnection); + aProtocol->TellThreadToDie(PR_TRUE); + retVal = PR_TRUE; + } + } + PR_CExitMonitor(this); + return retVal; + } + nsresult nsImapIncomingServer::CreateImapConnection(nsIEventQueue *aEventQueue, nsIImapUrl * aImapUrl, *************** *** 274,284 **** *aImapConnection = nsnull; // iterate through the connection cache for a connection that can handle this url. PRUint32 cnt; rv = m_connectionCache->Count(&cnt); if (NS_FAILED(rv)) return rv; for (PRUint32 i = 0; i < cnt && !canRunUrl && !hasToWait; i++) { ! connection = do_QueryInterface(m_connectionCache->ElementAt(i)); if (connection) connection->CanHandleUrl(aImapUrl, canRunUrl, hasToWait); --- 316,329 ---- *aImapConnection = nsnull; // iterate through the connection cache for a connection that can handle this url. PRUint32 cnt; + nsCOMPtr<nsISupports> aSupport; + rv = m_connectionCache->Count(&cnt); if (NS_FAILED(rv)) return rv; for (PRUint32 i = 0; i < cnt && !canRunUrl && !hasToWait; i++) { ! aSupport = getter_AddRefs(m_connectionCache->ElementAt(i)); ! connection = do_QueryInterface(aSupport); if (connection) connection->CanHandleUrl(aImapUrl, canRunUrl, hasToWait); *************** *** 289,294 **** --- 334,344 ---- freeConnection = connection; } } + + if (ConnectionTimeOut(connection)) + connection = null_nsCOMPtr(); + if (ConnectionTimeOut(freeConnection)) + freeConnection = null_nsCOMPtr(); // if we got here and we have a connection, then we should return it! if (canRunUrl && connection) Index: src/nsImapProtocol.cpp =================================================================== RCS file: /cvsroot/mozilla/mailnews/imap/src/nsImapProtocol.cpp,v retrieving revision 1.78 diff -c -r1.78 nsImapProtocol.cpp *** nsImapProtocol.cpp 1999/05/19 04:43:00 1.78 --- nsImapProtocol.cpp 1999/05/20 13:47:55 *************** *** 204,209 **** --- 204,210 ---- m_trackingTime = PR_FALSE; LL_I2L(m_startTime, 0); LL_I2L(m_endTime, 0); + LL_I2L(m_lastActiveTime, 0); m_tooFastTime = 0; m_idealTime = 0; m_chunkAddSize = 0; *************** *** 542,547 **** --- 543,563 ---- aImapServer->RemoveConnection(me); } + me->m_runningUrl = null_nsCOMPtr(); + me->m_transport = null_nsCOMPtr(); + me->m_inputStream = null_nsCOMPtr(); + me->m_outputStream = null_nsCOMPtr(); + me->m_outputConsumer = null_nsCOMPtr(); + me->m_displayConsumer = null_nsCOMPtr(); + me->m_sinkEventQueue = null_nsCOMPtr(); + me->m_eventQueue = null_nsCOMPtr(); + me->m_server = null_nsCOMPtr(); + me->m_imapLog = null_nsCOMPtr(); + me->m_imapMailFolderSink = null_nsCOMPtr(); + me->m_imapExtensionSink = null_nsCOMPtr(); + me->m_imapMessageSink = null_nsCOMPtr(); + me->m_imapMiscellaneousSink = null_nsCOMPtr(); + NS_RELEASE(me); } *************** *** 592,606 **** NS_IMETHODIMP nsImapProtocol::TellThreadToDie(PRBool isSafeToDie) { // **** jt - This routine should only be called by imap service. static char logoutString[] = "???? logout\r\n"; ! SendData(logoutString); // GetServerStateParser().ParseIMAPServerResponse(logoutString); - m_transport = null_nsCOMPtr(); - m_outputStream = null_nsCOMPtr(); - m_outputConsumer = null_nsCOMPtr(); - PR_EnterMonitor(m_threadDeathMonitor); m_threadShouldDie = PR_TRUE; PR_ExitMonitor(m_threadDeathMonitor); --- 608,620 ---- NS_IMETHODIMP nsImapProtocol::TellThreadToDie(PRBool isSafeToDie) { + PR_CEnterMonitor(this); // **** jt - This routine should only be called by imap service. static char logoutString[] = "???? logout\r\n"; ! if (m_transport) ! SendData(logoutString); // GetServerStateParser().ParseIMAPServerResponse(logoutString); PR_EnterMonitor(m_threadDeathMonitor); m_threadShouldDie = PR_TRUE; PR_ExitMonitor(m_threadDeathMonitor); *************** *** 613,621 **** --- 627,647 ---- PR_NotifyAll(m_urlReadyToRunMonitor); PR_ExitMonitor(m_urlReadyToRunMonitor); + PR_CExitMonitor(this); + return NS_OK; } + NS_IMETHODIMP + nsImapProtocol::GetLastActiveTimeStamp(PRTime* aTimeStamp) + { + PR_CEnterMonitor(this); + if (aTimeStamp) + *aTimeStamp = m_lastActiveTime; + PR_CExitMonitor(this); + return NS_OK; + } + void nsImapProtocol::ImapThreadMainLoop() { *************** *** 842,848 **** else if (!logonFailed) HandleCurrentUrlError(); ! m_runningUrl->SetUrlState(PR_FALSE, NS_OK); // we are done with this url. PseudoInterrupt(FALSE); // clear this, because we must be done interrupting? // release the url as we are done with it... --- 868,876 ---- else if (!logonFailed) HandleCurrentUrlError(); ! if (m_runningUrl) ! m_runningUrl->SetUrlState(PR_FALSE, NS_OK); // we are done with this url. ! m_lastActiveTime = PR_Now(); // ** jt -- is this the best place for time stamp PseudoInterrupt(FALSE); // clear this, because we must be done interrupting? // release the url as we are done with it... *************** *** 901,906 **** --- 929,937 ---- { if (m_runningUrl) m_runningUrl->SetUrlState(PR_FALSE, aStatus); // set change in url + m_transport = null_nsCOMPtr(); + m_outputStream = null_nsCOMPtr(); + m_outputConsumer = null_nsCOMPtr(); return NS_OK; } *************** *** 971,977 **** rv = SetupWithUrl(aURL, aConsumer); if (NS_FAILED(rv)) return rv; SetupSinkProxy(); // generate proxies for all of the event sinks in the url ! if (m_transport && m_runningUrl) { nsIImapUrl::nsImapAction imapAction; --- 1002,1008 ---- rv = SetupWithUrl(aURL, aConsumer); if (NS_FAILED(rv)) return rv; SetupSinkProxy(); // generate proxies for all of the event sinks in the url ! m_lastActiveTime = PR_Now(); if (m_transport && m_runningUrl) { nsIImapUrl::nsImapAction imapAction; Index: src/nsImapProtocol.h =================================================================== RCS file: /cvsroot/mozilla/mailnews/imap/src/nsImapProtocol.h,v retrieving revision 1.46 diff -c -r1.46 nsImapProtocol.h *** nsImapProtocol.h 1999/05/18 22:38:24 1.46 --- nsImapProtocol.h 1999/05/20 13:47:56 *************** *** 110,115 **** --- 110,117 ---- NS_IMETHOD GetDisplayStream (nsIWebShell **webShell); // Tell thread to die. This can only be called by imap service NS_IMETHOD TellThreadToDie(PRBool isSafeToDie); + // Get last active time stamp + NS_IMETHOD GetLastActiveTimeStamp(PRTime *aTimeStamp); //////////////////////////////////////////////////////////////////////////////// //////// // End of nsIStreamListenerSupport //////////////////////////////////////////////////////////////////////////////// //////// *************** *** 430,435 **** --- 432,438 ---- PRBool m_trackingTime; PRTime m_startTime; PRTime m_endTime; + PRTime m_lastActiveTime; PRInt32 m_tooFastTime; PRInt32 m_idealTime; PRInt32 m_chunkAddSize; Index: src/nsImapProxyEvent.cpp =================================================================== RCS file: /cvsroot/mozilla/mailnews/imap/src/nsImapProxyEvent.cpp,v retrieving revision 1.16 diff -c -r1.16 nsImapProxyEvent.cpp *** nsImapProxyEvent.cpp 1999/05/12 03:47:46 1.16 --- nsImapProxyEvent.cpp 1999/05/20 13:47:58 *************** *** 605,611 **** nsImapMessageSinkProxy::~nsImapMessageSinkProxy() { ! NS_IF_ADDREF (m_realImapMessageSink); } static NS_DEFINE_IID(kIImapMessageSinkIID, NS_IIMAPMESSAGESINK_IID); --- 605,611 ---- nsImapMessageSinkProxy::~nsImapMessageSinkProxy() { ! NS_IF_RELEASE (m_realImapMessageSink); } static NS_DEFINE_IID(kIImapMessageSinkIID, NS_IIMAPMESSAGESINK_IID); *************** *** 882,888 **** nsImapExtensionSinkProxy::~nsImapExtensionSinkProxy() { ! NS_IF_ADDREF (m_realImapExtensionSink); } static NS_DEFINE_IID(kIImapExtensionSinkIID, NS_IIMAPEXTENSIONSINK_IID); --- 882,888 ---- nsImapExtensionSinkProxy::~nsImapExtensionSinkProxy() { ! NS_IF_RELEASE (m_realImapExtensionSink); } static NS_DEFINE_IID(kIImapExtensionSinkIID, NS_IIMAPEXTENSIONSINK_IID); *************** *** 1121,1127 **** nsImapMiscellaneousSinkProxy::~nsImapMiscellaneousSinkProxy() { ! NS_IF_ADDREF (m_realImapMiscellaneousSink); } static NS_DEFINE_IID(kIImapMiscellaneousSinkIID, NS_IIMAPMISCELLANEOUSSINK_IID); --- 1121,1127 ---- nsImapMiscellaneousSinkProxy::~nsImapMiscellaneousSinkProxy() { ! NS_IF_RELEASE (m_realImapMiscellaneousSink); } static NS_DEFINE_IID(kIImapMiscellaneousSinkIID, NS_IIMAPMISCELLANEOUSSINK_IID); Index: src/nsImapUrl.cpp =================================================================== RCS file: /cvsroot/mozilla/mailnews/imap/src/nsImapUrl.cpp,v retrieving revision 1.36 diff -c -r1.36 nsImapUrl.cpp *** nsImapUrl.cpp 1999/05/07 21:30:39 1.36 --- nsImapUrl.cpp 1999/05/20 13:47:59 *************** *** 544,550 **** nsIImapIncomingServer::GetIID(), getter_AddRefs(servers)); if (NS_FAILED(rv)) return rv; ! nsCOMPtr<nsIMsgIncomingServer> server (do_QueryInterface(servers->ElementAt(0))); if (NS_FAILED(rv)) return rv; m_server = do_QueryInterface(server); } --- 544,552 ---- nsIImapIncomingServer::GetIID(), getter_AddRefs(servers)); if (NS_FAILED(rv)) return rv; ! nsCOMPtr<nsISupports> aSupport = ! getter_AddRefs(servers->ElementAt(0)); ! nsCOMPtr<nsIMsgIncomingServer> server (do_QueryInterface(aSupport)); if (NS_FAILED(rv)) return rv; m_server = do_QueryInterface(server); } Index: src/nsImapUtils.cpp =================================================================== RCS file: /cvsroot/mozilla/mailnews/imap/src/nsImapUtils.cpp,v retrieving revision 1.9 diff -c -r1.9 nsImapUtils.cpp *** nsImapUtils.cpp 1999/05/10 22:26:21 1.9 --- nsImapUtils.cpp 1999/05/20 13:48:00 *************** *** 47,54 **** getter_AddRefs(servers)); if (NS_FAILED(rv)) return rv; ! nsCOMPtr<nsIMsgIncomingServer> ! server(do_QueryInterface(servers->ElementAt(0))); char *localPath = nsnull; --- 47,54 ---- getter_AddRefs(servers)); if (NS_FAILED(rv)) return rv; ! nsCOMPtr<nsISupports> aSupport = getter_AddRefs(servers->ElementAt(0)); ! nsCOMPtr<nsIMsgIncomingServer> server(do_QueryInterface(aSupport)); char *localPath = nsnull; =============================================================================== ? networkPatch.txt ? socketPatch.txt Index: nsNetThread.cpp =================================================================== RCS file: /cvsroot/mozilla/network/module/nsNetThread.cpp,v retrieving revision 1.26 diff -c -r1.26 nsNetThread.cpp *** nsNetThread.cpp 1999/05/04 23:31:25 1.26 --- nsNetThread.cpp 1999/05/20 00:58:50 *************** *** 442,456 **** mProxy = aProxy; mURL = aURL; ! NS_ADDREF(mProxy); ! NS_ADDREF(mURL); } StreamListenerProxyEvent::~StreamListenerProxyEvent() { ! NS_RELEASE(mProxy); ! NS_RELEASE(mURL); } void StreamListenerProxyEvent::InitEvent() --- 442,456 ---- mProxy = aProxy; mURL = aURL; ! NS_IF_ADDREF(mProxy); ! NS_IF_ADDREF(mURL); } StreamListenerProxyEvent::~StreamListenerProxyEvent() { ! NS_IF_RELEASE(mProxy); ! NS_IF_RELEASE(mURL); } void StreamListenerProxyEvent::InitEvent()

I wish to check in for M6. It can be a blocker I think. What do you think, Lisa?

I'd argue that timeout is not an M6 stopper. I don't think it was ever in our M6 bucket anyway. It also implies that yout going to be running the app for over 30 minutes w/out something else crashing us. I guess my feeling is that this is adding a new feature (ability to handle time outs). Maybe I'm just being stingy & overly cautious... I'd like to try it out for qa day or so b4 we added it anyways to see how it runs. I'm also curious why StreamListenerProxyEvent needed to start storing the url (I wonder mostly out of ignorance & not because I think it is wrong).

I would argue that we should prevent adding a feature. We are not in beta. As long as it's solid and adding the stability to the product, why hanging off a useful feature. On regards of StreamListenerProxyEvent, I didn't add any new code. I modified the code to be more crash resistant. I changed NS_ADDREF() to NS_IF_ADDREF() and NS_RELEASE() to NS_IF_RELEASE() to prevent crashes. By the way the network/module will go away when Necko lands. We gain more stability from the proposed changes.

Although the fix is more than addressing the crash problem, it also allow you to create new connection and do useful things after the previous connection has been terminated by the server. It's up to you, Chris, whether we should take the fix or not. If you decide that we should move this bug to M7 please do so. Thanks much.

*** Bug 6878 has been marked as a duplicate of this bug. ***

lets move to m7

<will add to Verah's release notes for M6>

Code checked in for M7.

Verified fixed, 5_28 (M7) Mac, Win32, Linux. The connection is now silently reestablished to process the next user request after a server timeout.