nettika/geary
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Tidy up IMAP folder session management.

Browse source 
* src/engine/imap-engine/imap-engine-generic-account.vala
  (GenericAccount): Don't attempt to claim two client sessions when
  opening a folder, rather claim one and use as both an account session
  and a folder session. Rename open_folder_session to
  claim_folder_session to be consistent with account session API, update
  call sites.

* src/engine/imap-engine/imap-engine-minimal-folder.vala (MinimalFolder):
  Ensure the remote session is automatically re-established if
  appropriate on disconnect. Tidy up debug output and some comments.
This commit is contained in:
Michael James Gratton 2018-02-27 01:17:35 +11:00
parent 9cedfaf0d8
commit 9070380fc8
4 changed files with 52 additions and 27 deletions
Download patch file Download diff file Expand all files Collapse all files

2
src/engine/imap-engine/gmail/imap-engine-gmail-folder.vala
View file

@ -70,7 +70,7 @@ private class Geary.ImapEngine.GmailFolder : MinimalFolder, FolderSupport.Archiv
// separate connection and is not synchronized with the database, but also avoids a full
// folder normalization, which can be a heavyweight operation
GenericAccount account = (GenericAccount) folder.account;
Imap.FolderSession imap_trash = yield account.open_folder_session(
Imap.FolderSession imap_trash = yield account.claim_folder_session(
trash.path, cancellable
);
try {

43
src/engine/imap-engine/imap-engine-generic-account.vala
View file

@ -339,34 +339,51 @@ private abstract class Geary.ImapEngine.GenericAccount : Geary.Account {
}
/**
* Establishes a new IMAP folder session.
* Claims a new IMAP folder session from the pool.
*
* A new IMAP client session will be retrieved from the pool,
* connecting if needed, and used for a new folder session. This
* call will wait until the pool is ready to provide sessions. The
* session must be returned via {@link release_folder_session}
* after use.
*
* The account must have been opened before calling this method.
*/
public async Imap.FolderSession open_folder_session(Geary.FolderPath path,
Cancellable cancellable)
public async Imap.FolderSession claim_folder_session(Geary.FolderPath path,
Cancellable cancellable)
throws Error {
check_open();
debug("%s: Opening account session", this.to_string());
Imap.ClientSession? client = null;
debug("%s: Acquiring folder session", this.to_string());
yield this.remote_ready_lock.wait_async(cancellable);
// We manually construct an account session here and then
// reuse it for the folder session so we only need to claim as
// single session from the pool, not two.
Imap.ClientSession? client =
yield this.session_pool.claim_authorized_session_async(cancellable);
Imap.AccountSession account = new Imap.AccountSession(
this.information.id, client
);
Imap.Folder? folder = null;
GLib.Error? folder_err = null;
try {
// Do the claim_account_session first ensure the pool is
// ready.
Imap.AccountSession account = yield claim_account_session();
folder = yield account.fetch_folder_async(path, cancellable);
client = yield this.session_pool.claim_authorized_session_async(
cancellable
);
} catch (Error err) {
if (client != null) {
folder_err = err;
}
account.close();
if (folder_err != null) {
try {
yield this.session_pool.release_session_async(client);
} catch (Error release_err) {
debug("Error releasing folder session: %s", release_err.message);
}
throw err;
throw folder_err;
}
return yield new Imap.FolderSession(

32
src/engine/imap-engine/imap-engine-minimal-folder.vala
View file

@ -297,7 +297,7 @@ private class Geary.ImapEngine.MinimalFolder : Geary.Folder, Geary.FolderSupport
public async Imap.FolderSession claim_remote_session(Cancellable? cancellable = null)
throws Error {
check_open("claim_remote_session");
debug("%s: Acquiring folder session", this.to_string());
debug("%s: Claiming folder session", this.to_string());
yield this.wait_for_remote_async(cancellable);
return this.remote_session;
}
@ -776,7 +776,6 @@ private class Geary.ImapEngine.MinimalFolder : Geary.Folder, Geary.FolderSupport
private async void close_internal_locked(Folder.CloseReason local_reason,
Folder.CloseReason remote_reason,
Cancellable? cancellable) {
debug("%s: Closing", this.to_string());
// Ensure we don't attempt to start opening a remote while
// closing
this._account.session_pool.ready.disconnect(on_remote_ready);
@ -894,7 +893,7 @@ private class Geary.ImapEngine.MinimalFolder : Geary.Folder, Geary.FolderSupport
Imap.FolderSession? session = null;
try {
session = yield this._account.open_folder_session(this.path, cancellable);
session = yield this._account.claim_folder_session(this.path, cancellable);
} catch (Error err) {
if (!(err is IOError.CANCELLED)) {
// Notify that there was a connection error, but don't
@ -928,9 +927,10 @@ private class Geary.ImapEngine.MinimalFolder : Geary.Folder, Geary.FolderSupport
try {
yield normalize_folders(session, cancellable);
} catch (Error err) {
// Normalisation failed, which is also a serious problem
// so treat as in the error case above, after resolving if
// the issue was local or remote.
// Normalisation failed, so we have a pretty serious
// problem and should not try to use the folder further,
// unless the open was simply cancelled. So clean up, and
// force the folder closed.
this._account.release_folder_session(session);
if (!(err is IOError.CANCELLED)) {
Folder.CloseReason local_reason = CloseReason.LOCAL_ERROR;
@ -960,10 +960,8 @@ private class Geary.ImapEngine.MinimalFolder : Geary.Folder, Geary.FolderSupport
session.folder.properties, cancellable
);
} catch (Error err) {
// Database failed, so we have a pretty serious problem
// and should not try to use the folder further, unless
// the open was simply cancelled. So clean up, and force
// the folder closed if needed.
// Database failed, which is also a pretty serious
// problem, so handle as per above.
this._account.release_folder_session(session);
if (!(err is IOError.CANCELLED)) {
notify_open_failed(Folder.OpenFailed.LOCAL_ERROR, err);
@ -999,7 +997,7 @@ private class Geary.ImapEngine.MinimalFolder : Geary.Folder, Geary.FolderSupport
// folder to open that the result of that operation is ready
notify_remote_waiters(true);
// Update flags once the folder has opened. We will receive
// Update flags once the remote has opened. We will receive
// notifications of changes as long as the session remains
// open, so only need to do this once
this.update_flags_timer.start();
@ -1510,12 +1508,22 @@ private class Geary.ImapEngine.MinimalFolder : Geary.Folder, Geary.FolderSupport
}
private void on_remote_disconnected(Imap.ClientSession.DisconnectReason reason) {
bool is_error = reason.is_error();
// Need to close the remote session immediately to avoid a
// race with it opening again
Geary.Folder.CloseReason remote_reason = reason.is_error()
Geary.Folder.CloseReason remote_reason = is_error
? Geary.Folder.CloseReason.REMOTE_ERROR
: Geary.Folder.CloseReason.REMOTE_CLOSE;
close_remote_session(remote_reason);
// If an error occurred, but the folder is still open and so
// is the pool, try re-establishing the connection.
if (is_error &&
this._account.session_pool.is_ready &&
!this.open_cancellable.is_cancelled()) {
this.open_remote_session.begin();
}
}
}

2
src/engine/imap-engine/imap-engine-revokable-committed-move.vala
View file

@ -28,7 +28,7 @@ private class Geary.ImapEngine.RevokableCommittedMove : Revokable {
try {
// use a detached folder to quickly open, issue command, and leave, without full
// normalization that MinimalFolder requires
session = yield this.account.open_folder_session(destination, cancellable);
session = yield this.account.claim_folder_session(destination, cancellable);
foreach (Imap.MessageSet msg_set in Imap.MessageSet.uid_sparse(destination_uids)) {
// don't use Cancellable to try to make operations atomic
yield session.copy_email_async(msg_set, source, null);