Bug 530264 - Copy From Container will not always correctly copy data

- add two methods to DockerConnection: getOperationToken() and
  closeOperationToken() which will return and close a DockerClient
  copy respectively
- add two new forms of copyContainer and copyToContainer methods
  in DockerConnection that take a Closeable operation token as
  the first argument
- change all callers of copyContainer and copyToContainer to get
  an operation token and then use the new versions of the methods
- as well make sure callers either use a special try (Closeable)
  or manually close the operation token upon completion of the
  operation

Change-Id: I151ee185bac2aa05ee0ef07ce1b9ccbf77987235
Reviewed-on: https://git.eclipse.org/r/116644
Tested-by: Hudson CI
Reviewed-by: Jeff Johnston <jjohnstn@redhat.com>

5 files changed, 63 insertions, 63 deletions

diff --git a/containers/org.eclipse.linuxtools.docker.core/src/org/eclipse/linuxtools/internal/docker/core/DockerConnection.java b/containers/org.eclipse.linuxtools.docker.core/src/org/eclipse/linuxtools/internal/docker/core/DockerConnection.java
index 842e098024..e7da2d8b4b 100644
--- a/containers/org.eclipse.linuxtools.docker.core/src/org/eclipse/linuxtools/internal/docker/core/DockerConnection.java
+++ b/containers/org.eclipse.linuxtools.docker.core/src/org/eclipse/linuxtools/internal/docker/core/DockerConnection.java
@@ -502,7 +502,7 @@ public class DockerConnection
 	 * 
 	 * @return copy of client
 	 * @throws DockerException
-	 * @throws DockerCertificateException
+	 *             - general Docker client exception
 	 * @see DockerConnection#open(boolean)
 	 */
 	private DockerClient getClientCopy() throws DockerException {
@@ -514,6 +514,15 @@ public class DockerConnection
 		}
 	}
 
+	public Closeable getOperationToken() throws DockerException {
+		return getClientCopy();
+	}
+
+	public void closeOperationToken(Object token) {
+		DockerClient client = (DockerClient) token;
+		client.close();
+	}
+
 	// TODO: we might need something more fine grained, to indicate which
 	// container changed, was added or was removed, so we can refresh the UI
 	// accordingly.
@@ -1833,6 +1842,19 @@ public class DockerConnection
 		return stream;
 	}
 
+	public InputStream copyContainer(final Closeable token,
+			final String id, final String path)
+			throws DockerException, InterruptedException {
+		InputStream stream;
+		DockerClient clientCopy = (DockerClient) token;
+		try {
+			stream = clientCopy.archiveContainer(id, path);
+		} catch (com.spotify.docker.client.exceptions.DockerException e) {
+			throw new DockerException(e.getMessage(), e.getCause());
+		}
+		return stream;
+	}
+
 	@Override
 	public List<IDockerContainerChange> containerChanges(final String id)
 			throws DockerException, InterruptedException {
@@ -1895,6 +1917,20 @@ public class DockerConnection
 		}
 	}
 
+	public void copyToContainer(final Closeable token, final String directory,
+			final String id, final String path)
+			throws DockerException, InterruptedException, IOException {
+		try {
+			DockerClient copy = (DockerClient) token;
+			java.nio.file.Path dirPath = FileSystems.getDefault()
+					.getPath(directory);
+			copy.copyToContainer(dirPath, id, path);
+			copy.close(); /* dispose of client copy now that we are done */
+		} catch (com.spotify.docker.client.exceptions.DockerException e) {
+			throw new DockerException(e.getMessage(), e.getCause());
+		}
+	}
+
 	@Override
 	public int auth(IRegistryAccount cfg)
 			throws DockerException, InterruptedException {
diff --git a/containers/org.eclipse.linuxtools.docker.ui/src/org/eclipse/linuxtools/docker/ui/launch/ContainerLauncher.java b/containers/org.eclipse.linuxtools.docker.ui/src/org/eclipse/linuxtools/docker/ui/launch/ContainerLauncher.java
index cc745da3fe..121c4787cb 100644
--- a/containers/org.eclipse.linuxtools.docker.ui/src/org/eclipse/linuxtools/docker/ui/launch/ContainerLauncher.java
+++ b/containers/org.eclipse.linuxtools.docker.ui/src/org/eclipse/linuxtools/docker/ui/launch/ContainerLauncher.java
@@ -12,6 +12,7 @@ package org.eclipse.linuxtools.docker.ui.launch;
 
 import java.io.BufferedReader;
 import java.io.BufferedWriter;
+import java.io.Closeable;
 import java.io.File;
 import java.io.FileInputStream;
 import java.io.FileNotFoundException;
@@ -264,14 +265,15 @@ public class ContainerLauncher {
 							continue;
 						}
 					}
-					try {
+					try (Closeable token = ((DockerConnection) connection)
+							.getOperationToken()) {
 						monitor.setTaskName(Messages.getFormattedString(
 								COPY_VOLUMES_FROM_TASK, volume));
 						monitor.worked(1);
 
 
 						InputStream in = ((DockerConnection) connection)
-								.copyContainer(containerId, volume);
+								.copyContainer(token, containerId, volume);
 
 						synchronized (lockObject) {
 							dirList.add(volume);
@@ -283,8 +285,7 @@ public class ContainerLauncher {
 						 * stream that is guaranteed to block until data is
 						 * available.
 						 */
-						TarArchiveInputStream k = new TarArchiveInputStream(
-								new BlockingInputStream(in));
+						TarArchiveInputStream k = new TarArchiveInputStream(in);
 						TarArchiveEntry te = null;
 						target.toFile().mkdirs();
 						IPath currDir = target.append(volume)
@@ -344,22 +345,6 @@ public class ContainerLauncher {
 		}
 	}
 
-	/**
-	 * A blocking input stream that waits until data is available.
-	 */
-	private class BlockingInputStream extends InputStream {
-		private InputStream in;
-
-		public BlockingInputStream(InputStream in) {
-			this.in = in;
-		}
-
-		@Override
-		public int read() throws IOException {
-			return in.read();
-		}
-	}
-
 	@Override
 	protected void finalize() throws Throwable {
 		synchronized (lockObject) {
diff --git a/containers/org.eclipse.linuxtools.docker.ui/src/org/eclipse/linuxtools/internal/docker/ui/commands/CopyFromContainerCommandHandler.java b/containers/org.eclipse.linuxtools.docker.ui/src/org/eclipse/linuxtools/internal/docker/ui/commands/CopyFromContainerCommandHandler.java
index a6ad0806a3..bc25be0a89 100644
--- a/containers/org.eclipse.linuxtools.docker.ui/src/org/eclipse/linuxtools/internal/docker/ui/commands/CopyFromContainerCommandHandler.java
+++ b/containers/org.eclipse.linuxtools.docker.ui/src/org/eclipse/linuxtools/internal/docker/ui/commands/CopyFromContainerCommandHandler.java
@@ -10,6 +10,7 @@
  *******************************************************************************/
 package org.eclipse.linuxtools.internal.docker.ui.commands;
 
+import java.io.Closeable;
 import java.io.File;
 import java.io.FileOutputStream;
 import java.io.IOException;
@@ -98,7 +99,8 @@ public class CopyFromContainerCommandHandler extends AbstractHandler {
 				monitor.beginTask(
 						CommandMessages.getString(COPY_FROM_CONTAINER_JOB_TASK),
 						files.size());
-				try {
+				try (Closeable token = ((DockerConnection) connection)
+						.getOperationToken()) {
 					for (ContainerFileProxy proxy : files) {
 						if (monitor.isCanceled()) {
 							monitor.done();
@@ -111,7 +113,7 @@ public class CopyFromContainerCommandHandler extends AbstractHandler {
 											proxy.getFullPath()));
 							monitor.worked(1);
 							InputStream in = ((DockerConnection) connection)
-									.copyContainer(container.id(),
+									.copyContainer(token, container.id(),
 											proxy.getLink());
 							/*
 							 * The input stream from copyContainer might be
@@ -120,7 +122,7 @@ public class CopyFromContainerCommandHandler extends AbstractHandler {
 							 * data is available.
 							 */
 							TarArchiveInputStream k = new TarArchiveInputStream(
-									new BlockingInputStream(in));
+									in);
 							TarArchiveEntry te = null;
 							while ((te = k.getNextTarEntry()) != null) {
 								long size = te.getSize();
@@ -170,6 +172,8 @@ public class CopyFromContainerCommandHandler extends AbstractHandler {
 					// do nothing
 				} catch (IOException e) {
 					Activator.log(e);
+				} catch (DockerException e1) {
+					Activator.log(e1);
 				} finally {
 					monitor.done();
 				}
@@ -179,23 +183,6 @@ public class CopyFromContainerCommandHandler extends AbstractHandler {
 		};
 
 		copyFromContainerJob.schedule();
-
-	}
-
-	/**
-	 * A blocking input stream that waits until data is available.
-	 */
-	public class BlockingInputStream extends InputStream {
-		private InputStream in;
-
-		public BlockingInputStream(InputStream in) {
-			this.in = in;
-		}
-
-		@Override
-		public int read() throws IOException {
-			return in.read();
-		}
 	}
 
 }
diff --git a/containers/org.eclipse.linuxtools.docker.ui/src/org/eclipse/linuxtools/internal/docker/ui/commands/CopyToContainerCommandHandler.java b/containers/org.eclipse.linuxtools.docker.ui/src/org/eclipse/linuxtools/internal/docker/ui/commands/CopyToContainerCommandHandler.java
index 67f682773f..7ead076a92 100644
--- a/containers/org.eclipse.linuxtools.docker.ui/src/org/eclipse/linuxtools/internal/docker/ui/commands/CopyToContainerCommandHandler.java
+++ b/containers/org.eclipse.linuxtools.docker.ui/src/org/eclipse/linuxtools/internal/docker/ui/commands/CopyToContainerCommandHandler.java
@@ -10,6 +10,7 @@
  *******************************************************************************/
 package org.eclipse.linuxtools.internal.docker.ui.commands;
 
+import java.io.Closeable;
 import java.io.File;
 import java.io.IOException;
 import java.nio.file.FileSystems;
@@ -99,7 +100,8 @@ public class CopyToContainerCommandHandler extends AbstractHandler {
 						CommandMessages.getString(COPY_TO_CONTAINER_JOB_TASK),
 						files.size() + 1);
 				java.nio.file.Path tmpDir = null;
-				try {
+				try (Closeable token = ((DockerConnection) connection)
+						.getOperationToken()) {
 					for (Object proxy : files) {
 						File file = (File) proxy;
 						if (monitor.isCanceled()) {
@@ -188,7 +190,8 @@ public class CopyToContainerCommandHandler extends AbstractHandler {
 					// via the temporary directory
 					try {
 						((DockerConnection) connection).copyToContainer(
-								tmpDir.toString(), container.id(), target);
+								token, tmpDir.toString(), container.id(),
+								target);
 						deleteTmpDir(tmpDir);
 					} catch (final DockerException | IOException e) {
 						Display.getDefault()
@@ -206,6 +209,10 @@ public class CopyToContainerCommandHandler extends AbstractHandler {
 					}
 				} catch (InterruptedException e) {
 					// do nothing
+				} catch (IOException e1) {
+					Activator.log(e1);
+				} catch (DockerException e1) {
+					Activator.log(e1);
 				} finally {
 					monitor.done();
 				}
diff --git a/containers/org.eclipse.linuxtools.docker.ui/src/org/eclipse/linuxtools/internal/docker/ui/launch/ContainerCommandProcess.java b/containers/org.eclipse.linuxtools.docker.ui/src/org/eclipse/linuxtools/internal/docker/ui/launch/ContainerCommandProcess.java
index e73bc59cf0..3230133b1f 100644
--- a/containers/org.eclipse.linuxtools.docker.ui/src/org/eclipse/linuxtools/internal/docker/ui/launch/ContainerCommandProcess.java
+++ b/containers/org.eclipse.linuxtools.docker.ui/src/org/eclipse/linuxtools/internal/docker/ui/launch/ContainerCommandProcess.java
@@ -11,6 +11,7 @@
 package org.eclipse.linuxtools.internal.docker.ui.launch;
 
 import java.io.ByteArrayOutputStream;
+import java.io.Closeable;
 import java.io.File;
 import java.io.FileOutputStream;
 import java.io.IOException;
@@ -166,22 +167,6 @@ public class ContainerCommandProcess extends Process {
 		return new ByteArrayOutputStream();
 	}
 
-	/**
-	 * A blocking input stream that waits until data is available.
-	 */
-	private class BlockingInputStream extends InputStream {
-		private InputStream in;
-
-		public BlockingInputStream(InputStream in) {
-			this.in = in;
-		}
-
-		@Override
-		public int read() throws IOException {
-			return in.read();
-		}
-	}
-
 	private class CopyVolumesFromImageJob extends Job {
 
 		private static final String COPY_VOLUMES_FROM_JOB_TITLE = "ContainerLaunch.copyVolumesFromJob.title"; //$NON-NLS-1$
@@ -214,13 +199,14 @@ public class ContainerCommandProcess extends Process {
 				containerId = ((DockerConnection) connection)
 						.createContainer(config, hostConfig, null);
 				for (String volume : remoteVolumes.keySet()) {
-					try {
+					try (Closeable token = ((DockerConnection) connection)
+							.getOperationToken()) {
 						monitor.setTaskName(Messages.getFormattedString(
 								COPY_VOLUMES_FROM_TASK, volume));
 						monitor.worked(1);
 
 						InputStream in = ((DockerConnection) connection)
-								.copyContainer(containerId,
+								.copyContainer(token, containerId,
 										remoteVolumes.get(volume));
 
 						/*
@@ -229,8 +215,7 @@ public class ContainerCommandProcess extends Process {
 						 * stream that is guaranteed to block until data is
 						 * available.
 						 */
-						TarArchiveInputStream k = new TarArchiveInputStream(
-								new BlockingInputStream(in));
+						TarArchiveInputStream k = new TarArchiveInputStream(in);
 						TarArchiveEntry te = null;
 						IPath currDir = new Path(volume).removeLastSegments(1);
 						currDir.toFile().mkdirs();