From dd3f48a7f785d08f9bf821ce2efb7566c376528b Mon Sep 17 00:00:00 2001
From: bontric <benjohnwie@gmail.com>
Date: Wed, 12 Sep 2018 18:37:31 +0200
Subject: [PATCH] Fix session termination on error
---
.../mailbox/AbstractMailboxSession.java | 21 ++++++++++++-------
.../bramble/mailbox/MailboxOwnerSession.java | 19 ++++++++++++-----
.../mailbox/PrivateMailboxSession.java | 10 ++++++++-
.../mailbox/protocol/MailboxProtocol.java | 10 +++++++--
.../MailboxProtocolIntegrationTest.java | 16 +++++++-------
5 files changed, 54 insertions(+), 22 deletions(-)
diff --git a/bramble-core/src/main/java/org/briarproject/bramble/mailbox/AbstractMailboxSession.java b/bramble-core/src/main/java/org/briarproject/bramble/mailbox/AbstractMailboxSession.java
index 5f27522d8..ca7c10d7b 100644
--- a/bramble-core/src/main/java/org/briarproject/bramble/mailbox/AbstractMailboxSession.java
+++ b/bramble-core/src/main/java/org/briarproject/bramble/mailbox/AbstractMailboxSession.java
@@ -22,6 +22,7 @@ import java.io.ByteArrayOutputStream;
import java.io.EOFException;
import java.io.IOException;
import java.io.InputStream;
+import java.net.ProtocolException;
import java.util.concurrent.Executor;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.logging.Logger;
@@ -135,16 +136,20 @@ abstract class AbstractMailboxSession {
while (!terminated.get()) {
try {
MailboxRequest req = mailboxProtocol.getNextRequest();
- if (req.getType() == END) {
- synchronized (remoteSessionFinished) {
- remoteSessionFinished.set(true);
- }
+ if (req.getType() == END)
return;
- }
ioExecutor.execute(() -> handleRequest(req));
} catch (InterruptedException e) {
- logException(LOG, INFO, e);
- return;
+ if (LOG.isLoggable(INFO))
+ LOG.info(e.toString());
+ } catch (ProtocolException e) {
+ if (LOG.isLoggable(INFO))
+ LOG.info(e.toString());
+ handleProtocolException();
+ } finally {
+ synchronized (remoteSessionFinished){
+ remoteSessionFinished.set(true);
+ }
}
}
}
@@ -277,6 +282,8 @@ abstract class AbstractMailboxSession {
public abstract void run() throws IOException;
+ protected abstract void handleProtocolException();
+
protected class MailboxSessionHandleException extends Exception {
public MailboxSessionHandleException(String error) {
super(error);
diff --git a/bramble-core/src/main/java/org/briarproject/bramble/mailbox/MailboxOwnerSession.java b/bramble-core/src/main/java/org/briarproject/bramble/mailbox/MailboxOwnerSession.java
index caed520d4..c794731a9 100644
--- a/bramble-core/src/main/java/org/briarproject/bramble/mailbox/MailboxOwnerSession.java
+++ b/bramble-core/src/main/java/org/briarproject/bramble/mailbox/MailboxOwnerSession.java
@@ -13,15 +13,13 @@ import java.io.IOException;
import java.util.concurrent.Executor;
import java.util.logging.Logger;
-import static java.util.logging.Level.INFO;
-
class MailboxOwnerSession extends AbstractMailboxSession {
private static final Logger LOG =
Logger.getLogger(MailboxOwnerSession.class.getName());
private final Executor ioExecutor;
private final MailboxProtocol mailboxProtocol;
- private SyncSession syncSession;
+ private SyncSession duplexOutgoingSession;
public MailboxOwnerSession(ContactId contactId, Executor ioExecutor,
KeyManager keyManager,
@@ -42,8 +40,19 @@ class MailboxOwnerSession extends AbstractMailboxSession {
@Override
public void run() throws IOException {
ioExecutor.execute(() -> readRequests());
- syncSession = createDuplexOutgoingSession();
- syncSession.run();
+ duplexOutgoingSession = createDuplexOutgoingSession();
+ duplexOutgoingSession.run();
+ try {
+ endSession();
+ } catch (InterruptedException e) {
+ throw new IOException("Interrupted while ending session");
+ }
+ }
+
+ @Override
+ protected void handleProtocolException() {
+ if (duplexOutgoingSession != null)
+ duplexOutgoingSession.interrupt();
}
@Override
diff --git a/bramble-core/src/main/java/org/briarproject/bramble/mailbox/PrivateMailboxSession.java b/bramble-core/src/main/java/org/briarproject/bramble/mailbox/PrivateMailboxSession.java
index cef10a8b6..9085b834b 100644
--- a/bramble-core/src/main/java/org/briarproject/bramble/mailbox/PrivateMailboxSession.java
+++ b/bramble-core/src/main/java/org/briarproject/bramble/mailbox/PrivateMailboxSession.java
@@ -25,6 +25,7 @@ class PrivateMailboxSession extends AbstractMailboxSession {
private static final Logger LOG =
Logger.getLogger(PrivateMailboxSession.class.getName());
private MailboxProtocol mailboxProtocol;
+ private SyncSession duplexOutgoingSession;
public PrivateMailboxSession(ContactId contactId, Executor ioExecutor,
KeyManager keyManager,
@@ -71,7 +72,8 @@ class PrivateMailboxSession extends AbstractMailboxSession {
@Override
public void run() throws IOException {
ioExecutor.execute(() -> super.readRequests());
- createDuplexOutgoingSession().run();
+ duplexOutgoingSession = createDuplexOutgoingSession();
+ duplexOutgoingSession.run();
try {
endSession();
} catch (InterruptedException e) {
@@ -79,6 +81,12 @@ class PrivateMailboxSession extends AbstractMailboxSession {
}
}
+ @Override
+ protected void handleProtocolException() {
+ if (duplexOutgoingSession != null)
+ duplexOutgoingSession.interrupt();
+ }
+
@Override
public void handleStore(MailboxRequestStore req)
throws MailboxSessionHandleException {
diff --git a/bramble-core/src/main/java/org/briarproject/bramble/mailbox/protocol/MailboxProtocol.java b/bramble-core/src/main/java/org/briarproject/bramble/mailbox/protocol/MailboxProtocol.java
index 7df043a43..e5db5b2f1 100644
--- a/bramble-core/src/main/java/org/briarproject/bramble/mailbox/protocol/MailboxProtocol.java
+++ b/bramble-core/src/main/java/org/briarproject/bramble/mailbox/protocol/MailboxProtocol.java
@@ -53,6 +53,8 @@ public class MailboxProtocol implements Runnable {
public void sendKeepAlive() throws IOException {
// flush the writer without pending data to send a keepalive
+ if (stopped.get())
+ throw new ProtocolException("Mailbox protocol has stopped");
synchronized (mailboxBdfWriter) {
mailboxBdfWriter.flush();
}
@@ -93,8 +95,12 @@ public class MailboxProtocol implements Runnable {
}
public MailboxRequest getNextRequest()
- throws InterruptedException {
- return inQueue.take();
+ throws InterruptedException, ProtocolException {
+ MailboxRequest req = inQueue.take();
+ if (stopped.get())
+ throw new ProtocolException("Protocol has stopped");
+
+ return req;
}
@Override
diff --git a/bramble-core/src/test/java/org/briarproject/bramble/mailbox/protocol/MailboxProtocolIntegrationTest.java b/bramble-core/src/test/java/org/briarproject/bramble/mailbox/protocol/MailboxProtocolIntegrationTest.java
index 7a5420289..d1e02bbbb 100644
--- a/bramble-core/src/test/java/org/briarproject/bramble/mailbox/protocol/MailboxProtocolIntegrationTest.java
+++ b/bramble-core/src/test/java/org/briarproject/bramble/mailbox/protocol/MailboxProtocolIntegrationTest.java
@@ -13,6 +13,7 @@ import org.junit.Test;
import java.io.IOException;
import java.io.PipedInputStream;
import java.io.PipedOutputStream;
+import java.net.ProtocolException;
import java.util.concurrent.BlockingQueue;
import java.util.concurrent.Executor;
import java.util.concurrent.RejectedExecutionHandler;
@@ -73,7 +74,7 @@ public class MailboxProtocolIntegrationTest extends BrambleTestCase {
mailboxProtocol = new MailboxProtocol(ioExecutor, bdfWriter, bdfReader);
}
- @Test
+ @Test(expected = ProtocolException.class)
public void testProtocolTerminationOnClosedConnection()
throws IOException, InterruptedException {
ioExecutor.execute(mailboxProtocol);
@@ -88,12 +89,13 @@ public class MailboxProtocolIntegrationTest extends BrambleTestCase {
pipedOS.close();
// And "End" request should be the next read request
- MailboxRequest recvReq = mailboxProtocol.getNextRequest();
- assertEquals(END, recvReq.getType());
-
- // pending request should be marked as failed
- assertEquals(false, req.awaitAndGetResponse());
- assertEquals("Connection closed", req.getError());
+ try {
+ MailboxRequest recvReq = mailboxProtocol.getNextRequest();
+ }finally {
+ // pending request should be marked as failed
+ assertEquals(false, req.awaitAndGetResponse());
+ assertEquals("Connection closed", req.getError());
+ }
}
--
GitLab