Split Command.serialize up into two phases to get IDLE logged properly

Command.serialize has now become send() and send_wait(). ClientSession
will signal a command has been sent after calling the first method, but
will not send another until the second returns. This allows IDLE and
AUTHENTICATE to stall the send queue until they are complete, but also
allows listeners to get signalled when the command has actually been
sent.
This commit is contained in:
Michael James Gratton 2018-07-20 10:45:05 +10:00
parent 74323d2935
commit 1bdfacc10a
4 changed files with 55 additions and 29 deletions

View file

@ -40,17 +40,20 @@ public class Geary.Imap.AuthenticateCommand : Command {
this(OAUTH2_METHOD, encoded_token);
}
/** Waits after serialisation has completed for authentication. */
public override async void serialize(Serializer ser,
GLib.Cancellable cancellable)
public override async void send(Serializer ser,
GLib.Cancellable cancellable)
throws GLib.Error {
yield base.serialize(ser, cancellable);
yield base.send(ser, cancellable);
this.serialised = true;
// Need to manually flush here since the connection will be
// waiting this to complete before do so.
yield ser.flush_stream(cancellable);
}
public override async void send_wait(Serializer ser,
GLib.Cancellable cancellable)
throws GLib.Error {
// Wait to either get a response or a continuation request
yield this.error_lock.wait_async(cancellable);
if (this.response_literal != null) {
@ -62,8 +65,8 @@ public class Geary.Imap.AuthenticateCommand : Command {
yield wait_until_complete(cancellable);
}
public override void cancel_serialization() {
base.cancel_serialization();
public override void cancel_send() {
base.cancel_send();
this.error_cancellable.cancel();
}
@ -82,7 +85,7 @@ public class Geary.Imap.AuthenticateCommand : Command {
} else {
if (this.method != AuthenticateCommand.OAUTH2_METHOD ||
this.response_literal != null) {
cancel_serialization();
cancel_send();
throw new ImapError.INVALID(
"Unexpected AUTHENTICATE continuation request"
);

View file

@ -63,8 +63,8 @@ public class Geary.Imap.Command : BaseObject {
/**
* The command's arguments as parameters.
*
* Subclassess may append arguments to this before {@link
* serialize} is called, ideally from the constructor.
* Subclassess may append arguments to this before {@link send} is
* called, ideally from their constructors.
*/
protected ListParameter args {
get; private set; default = new RootParameters();
@ -146,8 +146,8 @@ public class Geary.Imap.Command : BaseObject {
* required, this method will yield until a command continuation
* has been received, when it will resume the same process.
*/
public virtual async void serialize(Serializer ser,
GLib.Cancellable cancellable)
public virtual async void send(Serializer ser,
GLib.Cancellable cancellable)
throws GLib.Error {
this.response_timer.start();
this.tag.serialize(ser, cancellable);
@ -186,6 +186,25 @@ public class Geary.Imap.Command : BaseObject {
ser.push_eol(cancellable);
}
/**
* Check for command-specific server responses after sending.
*
* This method is called after {@link send} and after {@link
* ClientSession} has signalled the command has been sent, but
* before the next command is processed. It allows command
* implementations (e.g. {@link IdleCommand}) to asynchronously
* wait for some kind of response from the server before allowing
* additional commands to be sent.
*
* Most commands will not need to override this, and it by default
* does nothing.
*/
public virtual async void send_wait(Serializer ser,
GLib.Cancellable cancellable)
throws GLib.Error {
// Nothing to do by default
}
/**
* Cancels this command's execution.
*
@ -193,7 +212,7 @@ public class Geary.Imap.Command : BaseObject {
* including {@link wait_until_complete}.
*/
public virtual void cancel_command() {
cancel_serialization();
cancel_send();
this.response_timer.reset();
this.complete_lock.blind_notify();
}
@ -219,7 +238,7 @@ public class Geary.Imap.Command : BaseObject {
public virtual void completed(StatusResponse new_status)
throws ImapError {
if (this.status != null) {
cancel_serialization();
cancel_send();
throw new ImapError.SERVER_ERROR(
"%s: Duplicate status response received: %s",
to_brief_string(),
@ -230,7 +249,7 @@ public class Geary.Imap.Command : BaseObject {
this.status = new_status;
this.response_timer.reset();
this.complete_lock.blind_notify();
cancel_serialization();
cancel_send();
check_status();
}
@ -240,7 +259,7 @@ public class Geary.Imap.Command : BaseObject {
public virtual void data_received(ServerData data)
throws ImapError {
if (this.status != null) {
cancel_serialization();
cancel_send();
throw new ImapError.SERVER_ERROR(
"%s: Server data received when command already complete: %s",
to_brief_string(),
@ -255,14 +274,14 @@ public class Geary.Imap.Command : BaseObject {
* Called when a continuation was requested by the server.
*
* This will notify the command's literal spinlock so that if
* {@link serialize} is waiting to send a literal, it will do so
* {@link send} is waiting to send a literal, it will do so
* now.
*/
public virtual void
continuation_requested(ContinuationResponse continuation)
throws ImapError {
if (this.status != null) {
cancel_serialization();
cancel_send();
throw new ImapError.SERVER_ERROR(
"%s: Continuation requested when command already complete",
to_brief_string()
@ -270,7 +289,7 @@ public class Geary.Imap.Command : BaseObject {
}
if (this.literal_spinlock == null) {
cancel_serialization();
cancel_send();
throw new ImapError.SERVER_ERROR(
"%s: Continuation requested but no literals available",
to_brief_string()
@ -292,9 +311,9 @@ public class Geary.Imap.Command : BaseObject {
* Cancels any existing serialisation in progress.
*
* When this method is called, any non I/O related process
* blocking the blocking {@link serialize} must be cancelled.
* blocking the blocking {@link send} must be cancelled.
*/
protected virtual void cancel_serialization() {
protected virtual void cancel_send() {
if (this.literal_cancellable != null) {
this.literal_cancellable.cancel();
}

View file

@ -35,21 +35,25 @@ public class Geary.Imap.IdleCommand : Command {
}
/** Waits after serialisation has completed for {@link exit_idle}. */
public override async void serialize(Serializer ser,
GLib.Cancellable cancellable)
public override async void send(Serializer ser,
GLib.Cancellable cancellable)
throws GLib.Error {
// Need to manually flush here since Dovecot doesn't like
// getting IDLE in the same buffer as other commands.
yield ser.flush_stream(cancellable);
yield base.serialize(ser, cancellable);
yield base.send(ser, cancellable);
this.serialised = true;
// Need to manually flush again since the connection will be
// waiting this to complete before do so.
yield ser.flush_stream(cancellable);
}
// Now wait for exit_idle() to be called, the server to send a
public override async void send_wait(Serializer ser,
GLib.Cancellable cancellable)
throws GLib.Error {
// Wait for exit_idle() to be called, the server to send a
// status response, or everything to be cancelled.
yield this.exit_lock.wait_async(cancellable);
@ -68,8 +72,8 @@ public class Geary.Imap.IdleCommand : Command {
yield wait_until_complete(cancellable);
}
public override void cancel_serialization() {
base.cancel_serialization();
public override void cancel_send() {
base.cancel_send();
this.exit_cancellable.cancel();
}

View file

@ -463,7 +463,9 @@ public class Geary.Imap.ClientConnection : BaseObject {
this.current_command = command;
this.sent_queue.add(command);
yield command.serialize(this.ser, cancellable);
yield command.send(this.ser, cancellable);
sent_command(command);
yield command.send_wait(this.ser, cancellable);
} catch (GLib.Error err) {
ser_error = err;
}
@ -474,8 +476,6 @@ public class Geary.Imap.ClientConnection : BaseObject {
this.sent_queue.remove(command);
throw ser_error;
}
sent_command(command);
}
private Command? get_sent_command(Tag tag) {