Commit 6d6476e8 authored by David Faure's avatar David Faure
Browse files

Rework handling of connection failures, to fix crashes.

Testcase: unplugging my network cable, waiting for timeouts, plugging it
back again later, check mail.

* Port from deprecated connectionLost to stateChanged, which has the major
benefit of not confusing "failure to connect" with "we lost our connection"
(due to KIMAP::Session emitting both signals for compat reasons).

* Failure to connect is now handled in the result slot for the LoginJob,
rather than via the connectionFailed signal, since the first one is called
anyway.

* Make sure we emit connectDone on error, even if m_initialConnectDone is true,
so that the resource sets itself to offline (rather than it attempting the
same task over and over again!).

* Don't set the resource status to Broken when it's simply offline
(login failure due to "could not connect" is handled separately now)

* Make sure not to pop up a password dialog when couldn't connect; this
is different from "we could connect but the server refused login".

Requires kdepimlibs commit acdb0f7e50 for ERR_COULD_NOT_CONNECT.

FIXED-IN: 4.8
BUG: 288268
(cherry picked from commit 757f464e)
parent f9127a52
......@@ -263,6 +263,11 @@ void ImapResource::onConnectDone( int errorCode, const QString &errorString )
taskDone();
return;
case SessionPool::CouldNotConnectError:
setOnline( false );
taskDone();
return;
case SessionPool::ReconnectNeededError:
reconnect();
return;
......
......@@ -30,8 +30,6 @@
#include <kimap/capabilitiesjob.h>
#include <kimap/logoutjob.h>
#include <kimap/namespacejob.h>
#include <kimap/session.h>
#include <kimap/sessionuiproxy.h>
#include <kpimutils/networkaccesshelper.h>
......@@ -173,10 +171,8 @@ QList<KIMAP::MailBoxDescriptor> SessionPool::serverNamespaces() const
void SessionPool::killSession( KIMAP::Session *session, SessionTermination termination )
{
QObject::disconnect( session, SIGNAL(connectionLost()),
this, SLOT(onEarlyConnectionLost()) );
QObject::disconnect( session, SIGNAL(connectionLost()),
this, SLOT(onConnectionLost()) );
QObject::disconnect( session, SIGNAL(stateChanged(KIMAP::Session::State,KIMAP::Session::State)),
this, SLOT(onSessionStateChanged(KIMAP::Session::State,KIMAP::Session::State)) );
m_unusedPool.removeAll( session );
m_reservedPool.removeAll( session );
......@@ -195,11 +191,6 @@ void SessionPool::declareSessionReady( KIMAP::Session *session )
{
m_pendingInitialSession = 0;
QObject::disconnect( session, SIGNAL(connectionLost()),
this, SLOT(onEarlyConnectionLost()) );
QObject::connect( session, SIGNAL(connectionLost()),
this, SLOT(onConnectionLost()) );
if ( !m_initialConnectDone ) {
m_unusedPool << session;
m_initialConnectDone = true;
......@@ -220,17 +211,18 @@ void SessionPool::cancelSessionCreation( KIMAP::Session *session, int errorCode,
{
m_pendingInitialSession = 0;
if ( m_account ) {
emit connectDone( errorCode,
i18n( "Could not connect to the IMAP-server %1.\n%2",
m_account->server(), errorMessage ) );
} else {
// Can happen when we lose all ready connections while trying to establish
// a new connection, for example.
emit connectDone( errorCode,
i18n( "Cound not connect to the IMAP server.\n%1", errorMessage ) );
}
if ( !m_initialConnectDone ) {
if ( m_account ) {
emit connectDone( errorCode,
i18n( "Could not connect to the IMAP-server %1.\n%2",
m_account->server(), errorMessage ) );
} else {
// Can happen when we loose all ready connections while trying to establish
// a new connection, for example.
emit connectDone( errorCode,
i18n( "Cound not connect to the IMAP server.\n%1", errorMessage ) );
}
disconnect();
killSession( session, LogoutSession );
} else {
......@@ -331,7 +323,9 @@ void SessionPool::onPasswordRequestDone( int resultType, const QString &password
session->setUiProxy( m_sessionUiProxy );
session->setTimeout( m_account->timeout() );
}
QObject::connect( session, SIGNAL(connectionLost()), this, SLOT(onEarlyConnectionLost()) );
QObject::connect( session, SIGNAL(stateChanged(KIMAP::Session::State,KIMAP::Session::State)),
this, SLOT(onSessionStateChanged(KIMAP::Session::State,KIMAP::Session::State)) );
KIMAP::LoginJob *loginJob = new KIMAP::LoginJob( session );
loginJob->setUserName( m_account->userName() );
......@@ -344,14 +338,6 @@ void SessionPool::onPasswordRequestDone( int resultType, const QString &password
loginJob->start();
}
void SessionPool::onEarlyConnectionLost()
{
kDebug() << sender();
KIMAP::Session *session = qobject_cast<KIMAP::Session*>( sender() );
Q_ASSERT( session );
cancelSessionCreation( session, LoginFailError, i18n( "Failed to connect to server" ) );
}
void SessionPool::onLoginDone( KJob *job )
{
KIMAP::LoginJob *login = static_cast<KIMAP::LoginJob*>( job );
......@@ -366,13 +352,13 @@ void SessionPool::onLoginDone( KJob *job )
capJob->start();
}
} else {
//kWarning() << "Error during login:" << job->errorString();
if ( m_initialConnectDone ) {
if ( job->error() == KIMAP::LoginJob::ERR_COULD_NOT_CONNECT ) {
cancelSessionCreation( login->session(),
LoginFailError,
CouldNotConnectError,
i18n( "Could not connect to the IMAP-server %1.\n%2",
m_account->server(), job->errorString() ) );
} else {
// Connection worked, but login failed -> ask for a different password or ssl settings.
m_pendingInitialSession = login->session();
m_passwordRequester->requestPassword( PasswordRequesterInterface::WrongPasswordRequest,
job->errorString() );
......@@ -457,6 +443,13 @@ void SessionPool::onNamespacesTestDone( KJob *job )
declareSessionReady( nsJob->session() );
}
void SessionPool::onSessionStateChanged(KIMAP::Session::State newState, KIMAP::Session::State oldState)
{
if (newState == KIMAP::Session::Disconnected && oldState != KIMAP::Session::Disconnected) {
onConnectionLost();
}
}
void SessionPool::onConnectionLost()
{
KIMAP::Session *session = static_cast<KIMAP::Session*>( sender() );
......@@ -475,8 +468,7 @@ void SessionPool::onConnectionLost()
emit connectionLost( session );
//This next line seems like it can induce a crash
// session->deleteLater();
session->deleteLater();
}
#include "sessionpool.moc"
......@@ -26,12 +26,12 @@
#include <QtCore/QStringList>
#include <kimap/listjob.h>
#include <kimap/session.h>
#include <kimap/sessionuiproxy.h>
namespace KIMAP
{
class MailBoxDescriptor;
class Session;
}
namespace KPIMUtils {
......@@ -55,7 +55,8 @@ public:
LoginFailError,
CapabilitiesTestError,
IncompatibleServerError,
NoAvailableSessionError
NoAvailableSessionError,
CouldNotConnectError
};
enum SessionTermination {
......@@ -99,10 +100,10 @@ private slots:
void onCapabilitiesTestDone( KJob *job );
void onNamespacesTestDone( KJob *job );
void onConnectionLost();
void onEarlyConnectionLost();
void onSessionStateChanged(KIMAP::Session::State newState, KIMAP::Session::State oldState);
private:
void onConnectionLost();
void killSession( KIMAP::Session *session, SessionTermination termination );
void declareSessionReady( KIMAP::Session *session );
void cancelSessionCreation( KIMAP::Session *session, int errorCode, const QString &errorString );
......
......@@ -57,4 +57,37 @@ private slots:
void setupTestCase();
};
// Taken from Qt 5:
#if QT_VERSION < 0x050000
// Will try to wait for the expression to become true while allowing event processing
#define QTRY_VERIFY(__expr) \
do { \
const int __step = 50; \
const int __timeout = 5000; \
if (!(__expr)) { \
QTest::qWait(0); \
} \
for (int __i = 0; __i < __timeout && !(__expr); __i+=__step) { \
QTest::qWait(__step); \
} \
QVERIFY(__expr); \
} while (0)
// Will try to wait for the comparison to become successful while allowing event processing
#define QTRY_COMPARE(__expr, __expected) \
do { \
const int __step = 50; \
const int __timeout = 5000; \
if ((__expr) != (__expected)) { \
QTest::qWait(0); \
} \
for (int __i = 0; __i < __timeout && ((__expr) != (__expected)); __i+=__step) { \
QTest::qWait(__step); \
} \
QCOMPARE(__expr, __expected); \
} while (0)
#endif
#endif
......@@ -639,6 +639,35 @@ private slots:
server.quit();
}
void shouldNotifyFailureToConnect()
{
// This tests what happens when we can't connect to the server, e.g. due to being offline.
// In this test we just use 0.0.0.0 as an invalid server IP, instead.
ImapAccount *account = createDefaultAccount();
account->setServer("0.0.0.0"); // so that the connexion fails
DummyPasswordRequester *requester = createDefaultRequester();
QList<DummyPasswordRequester::RequestType> requests;
QList<DummyPasswordRequester::ResultType> results;
// I don't want to see "WrongPasswordRequest". A password popup is annoying when we're offline or the server is down.
requests << DummyPasswordRequester::StandardRequest;
results << DummyPasswordRequester::PasswordRetrieved;
requester->setScenario( requests, results );
QSignalSpy requesterSpy( requester, SIGNAL(done(int,QString)) );
SessionPool pool( 2 );
QSignalSpy connectDoneSpy( &pool, SIGNAL(connectDone(int,QString)) );
QSignalSpy lostSpy( &pool, SIGNAL(connectionLost(KIMAP::Session*)) );
QVERIFY( !pool.isConnected() );
pool.setPasswordRequester( requester );
pool.connect( account );
QVERIFY( !pool.isConnected() );
QTRY_COMPARE( requesterSpy.count(), requests.count() );
QTRY_COMPARE( connectDoneSpy.count(), 1 );
QCOMPARE( connectDoneSpy.at(0).at(0).toInt(), (int)SessionPool::CouldNotConnectError );
QCOMPARE( lostSpy.count(), 0 ); // don't want this, it makes the resource reconnect immediately (and fail, and reconnect, and so on...)
}
};
QTEST_KDEMAIN_CORE( TestSessionPool )
......
Supports Markdown
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment