UploadPack: don't prematurely terminate timer in case of error

In uploadWithExceptionPropagation don't prematurely terminate timer in
case of error to enable reporting it to the client. Expose a close
method so that callers can terminate it at the appropriate time.

If the timer is already terminated when trying to report it to the
client this failed with the error java.lang.IllegalStateException:
"Timer already terminated".

Bug: 579670
Change-Id: I95827442ccb0f9b1ede83630cf7c51cf619c399a

2 files changed, 21 insertions, 8 deletions

diff --git a/org.eclipse.jgit.http.server/src/org/eclipse/jgit/http/server/UploadPackServlet.java b/org.eclipse.jgit.http.server/src/org/eclipse/jgit/http/server/UploadPackServlet.java
index 2759b8a0fe..c4f845e52b 100644
--- a/org.eclipse.jgit.http.server/src/org/eclipse/jgit/http/server/UploadPackServlet.java
+++ b/org.eclipse.jgit.http.server/src/org/eclipse/jgit/http/server/UploadPackServlet.java
@@ -195,6 +195,8 @@ class UploadPackServlet extends HttpServlet {
 				log(up.getRepository(), e.getCause());
 				consumeRequestBody(req);
 				out.close();
+			} finally {
+				up.close();
 			}
 		};
 
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/UploadPack.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/UploadPack.java
index c389ce44f4..160e2e6e3d 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/UploadPack.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/UploadPack.java
@@ -39,6 +39,7 @@ import static org.eclipse.jgit.transport.GitProtocolConstants.VERSION_2_REQUEST;
 import static org.eclipse.jgit.util.RefMap.toRefMap;
 
 import java.io.ByteArrayOutputStream;
+import java.io.Closeable;
 import java.io.EOFException;
 import java.io.IOException;
 import java.io.InputStream;
@@ -104,7 +105,7 @@ import org.eclipse.jgit.util.io.TimeoutOutputStream;
 /**
  * Implements the server side of a fetch connection, transmitting objects.
  */
-public class UploadPack {
+public class UploadPack implements Closeable {
 	/** Policy the server uses to validate client requests */
 	public enum RequestPolicy {
 		/** Client may only ask for objects the server advertised a reference for. */
@@ -730,6 +731,17 @@ public class UploadPack {
 				&& clientRequestedV2;
 	}
 
+	@Override
+	public void close() {
+		if (timer != null) {
+			try {
+				timer.terminate();
+			} finally {
+				timer = null;
+			}
+		}
+	}
+
 	/**
 	 * Execute the upload task on the socket.
 	 *
@@ -777,6 +789,8 @@ public class UploadPack {
 				throw new UploadPackInternalServerErrorException(err);
 			}
 			throw err;
+		} finally {
+			close();
 		}
 	}
 
@@ -786,6 +800,10 @@ public class UploadPack {
 	 * <p>
 	 * If the client passed extra parameters (e.g., "version=2") through a side
 	 * channel, the caller must call setExtraParameters first to supply them.
+	 * Callers of this method should call {@link #close()} to terminate the
+	 * internal interrupt timer thread. If the caller fails to terminate the
+	 * thread, it will (eventually) terminate itself when the InterruptTimer
+	 * instance is garbage collected.
 	 *
 	 * @param input
 	 *            raw input to read client commands from. Caller must ensure the
@@ -842,13 +860,6 @@ public class UploadPack {
 		} finally {
 			msgOut = NullOutputStream.INSTANCE;
 			walk.close();
-			if (timer != null) {
-				try {
-					timer.terminate();
-				} finally {
-					timer = null;
-				}
-			}
 		}
 	}