[JGIT PATCH 4/6] Add timeouts to smart transport protocol servers

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Like with the client side support, we spawn a background thread and
use that to wake up the real service thread if it blocks too long in
a read or write operation.  Typically this sort of long running IO
indicates the client is not responding, and the server should abort
its transaction and disconnect the client.

Like with the push client, the push server doesn't know when the
client will be done computing its pack file and start sending it
to the server.  Consequently we aren't entirely sure when we can
safely say the client is dead, vs. the client is just busy doing
its local compression work before transmitting.  By upping our
timeout to 10x the originally configured value we can give the
client a reasonable chance to finish packing data before we do
wind up aborting.

Signed-off-by: Shawn O. Pearce <spearce@xxxxxxxxxxx>
---
 .../org/spearce/jgit/transport/ReceivePack.java    |   57 +++++++++++++++++++
 .../src/org/spearce/jgit/transport/UploadPack.java |   59 ++++++++++++++++++--
 2 files changed, 110 insertions(+), 6 deletions(-)

diff --git a/org.spearce.jgit/src/org/spearce/jgit/transport/ReceivePack.java b/org.spearce.jgit/src/org/spearce/jgit/transport/ReceivePack.java
index c92a903..16b0c57 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/transport/ReceivePack.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/transport/ReceivePack.java
@@ -69,6 +69,9 @@
 import org.spearce.jgit.revwalk.RevObject;
 import org.spearce.jgit.revwalk.RevWalk;
 import org.spearce.jgit.transport.ReceiveCommand.Result;
+import org.spearce.jgit.util.io.InterruptTimer;
+import org.spearce.jgit.util.io.TimeoutInputStream;
+import org.spearce.jgit.util.io.TimeoutOutputStream;
 
 /**
  * Implements the server side of a push connection, receiving objects.
@@ -109,6 +112,14 @@
 	/** Hook to report on the commands after execution. */
 	private PostReceiveHook postReceive;
 
+	/** Timeout in seconds to wait for client interaction. */
+	private int timeout;
+
+	/** Timer to manage {@link #timeout}. */
+	private InterruptTimer timer;
+
+	private TimeoutInputStream timeoutIn;
+
 	private InputStream rawIn;
 
 	private OutputStream rawOut;
@@ -297,6 +308,23 @@ public void setPostReceiveHook(final PostReceiveHook h) {
 		postReceive = h != null ? h : PostReceiveHook.NULL;
 	}
 
+	/** @return timeout (in seconds) before aborting an IO operation. */
+	public int getTimeout() {
+		return timeout;
+	}
+
+	/**
+	 * Set the timeout before willing to abort an IO call.
+	 *
+	 * @param seconds
+	 *            number of seconds to wait (with no data transfer occurring)
+	 *            before aborting an IO read or write operation with the
+	 *            connected client.
+	 */
+	public void setTimeout(final int seconds) {
+		timeout = seconds;
+	}
+
 	/** @return all of the command received by the current request. */
 	public List<ReceiveCommand> getAllCommands() {
 		return Collections.unmodifiableList(commands);
@@ -365,6 +393,17 @@ public void receive(final InputStream input, final OutputStream output,
 			rawIn = input;
 			rawOut = output;
 
+			if (timeout > 0) {
+				final Thread caller = Thread.currentThread();
+				timer = new InterruptTimer(caller.getName() + "-Timer");
+				timeoutIn = new TimeoutInputStream(rawIn, timer);
+				TimeoutOutputStream o = new TimeoutOutputStream(rawOut, timer);
+				timeoutIn.setTimeout(timeout * 1000);
+				o.setTimeout(timeout * 1000);
+				rawIn = timeoutIn;
+				rawOut = o;
+			}
+
 			pckIn = new PacketLineIn(rawIn);
 			pckOut = new PacketLineOut(rawOut);
 			if (messages != null) {
@@ -389,6 +428,7 @@ public void println() {
 				}
 			} finally {
 				unlockPack();
+				timeoutIn = null;
 				rawIn = null;
 				rawOut = null;
 				pckIn = null;
@@ -397,6 +437,13 @@ public void println() {
 				refs = null;
 				enabledCapablities = null;
 				commands = null;
+				if (timer != null) {
+					try {
+						timer.terminate();
+					} finally {
+						timer = null;
+					}
+				}
 			}
 		}
 	}
@@ -557,6 +604,13 @@ private boolean needPack() {
 	}
 
 	private void receivePack() throws IOException {
+		// It might take the client a while to pack the objects it needs
+		// to send to us.  We should increase our timeout so we don't
+		// abort while the client is computing.
+		//
+		if (timeoutIn != null)
+			timeoutIn.setTimeout(10 * timeout * 1000);
+
 		final IndexPack ip = IndexPack.create(db, rawIn);
 		ip.setFixThin(true);
 		ip.setObjectChecking(isCheckReceivedObjects());
@@ -566,6 +620,9 @@ private void receivePack() throws IOException {
 		if (getRefLogIdent() != null)
 			lockMsg += " from " + getRefLogIdent().toExternalString();
 		packLock = ip.renameAndOpenPack(lockMsg);
+
+		if (timeoutIn != null)
+			timeoutIn.setTimeout(timeout * 1000);
 	}
 
 	private void checkConnectivity() throws IOException {
diff --git a/org.spearce.jgit/src/org/spearce/jgit/transport/UploadPack.java b/org.spearce.jgit/src/org/spearce/jgit/transport/UploadPack.java
index 7d17b2d..b0fa885 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/transport/UploadPack.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/transport/UploadPack.java
@@ -64,6 +64,9 @@
 import org.spearce.jgit.revwalk.RevObject;
 import org.spearce.jgit.revwalk.RevTag;
 import org.spearce.jgit.revwalk.RevWalk;
+import org.spearce.jgit.util.io.InterruptTimer;
+import org.spearce.jgit.util.io.TimeoutInputStream;
+import org.spearce.jgit.util.io.TimeoutOutputStream;
 
 /**
  * Implements the server side of a fetch connection, transmitting objects.
@@ -89,6 +92,12 @@
 	/** Revision traversal support over {@link #db}. */
 	private final RevWalk walk;
 
+	/** Timeout in seconds to wait for client interaction. */
+	private int timeout;
+
+	/** Timer to manage {@link #timeout}. */
+	private InterruptTimer timer;
+
 	private InputStream rawIn;
 
 	private OutputStream rawOut;
@@ -164,6 +173,23 @@ public final RevWalk getRevWalk() {
 		return walk;
 	}
 
+	/** @return timeout (in seconds) before aborting an IO operation. */
+	public int getTimeout() {
+		return timeout;
+	}
+
+	/**
+	 * Set the timeout before willing to abort an IO call.
+	 *
+	 * @param seconds
+	 *            number of seconds to wait (with no data transfer occurring)
+	 *            before aborting an IO read or write operation with the
+	 *            connected client.
+	 */
+	public void setTimeout(final int seconds) {
+		timeout = seconds;
+	}
+
 	/**
 	 * Execute the upload task on the socket.
 	 *
@@ -183,12 +209,33 @@ public final RevWalk getRevWalk() {
 	 */
 	public void upload(final InputStream input, final OutputStream output,
 			final OutputStream messages) throws IOException {
-		rawIn = input;
-		rawOut = output;
+		try {
+			rawIn = input;
+			rawOut = output;
+
+			if (timeout > 0) {
+				final Thread caller = Thread.currentThread();
+				timer = new InterruptTimer(caller.getName() + "-Timer");
+				TimeoutInputStream i = new TimeoutInputStream(rawIn, timer);
+				TimeoutOutputStream o = new TimeoutOutputStream(rawOut, timer);
+				i.setTimeout(timeout * 1000);
+				o.setTimeout(timeout * 1000);
+				rawIn = i;
+				rawOut = o;
+			}
 
-		pckIn = new PacketLineIn(rawIn);
-		pckOut = new PacketLineOut(rawOut);
-		service();
+			pckIn = new PacketLineIn(rawIn);
+			pckOut = new PacketLineOut(rawOut);
+			service();
+		} finally {
+			if (timer != null) {
+				try {
+					timer.terminate();
+				} finally {
+					timer = null;
+				}
+			}
+		}
 	}
 
 	private void service() throws IOException {
-- 
1.6.3.2.416.g04d0

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]