Skip to content

Commit 822cb32

Browse files
author
Marcus Linke
committed
Fix issue docker-java#434 
1 parent 21c4825 commit 822cb32

File tree

9 files changed

+90
-68
lines changed

9 files changed

+90
-68
lines changed

‎src/main/java/com/github/dockerjava/api/async/ResultCallback.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,8 @@
77
*/
88
public interface ResultCallback<A_RES_T> extends Closeable {
99
/**
10-
* Called when the async processing starts. The passed {@link Closeable} can be used to close/interrupt the
11-
* processing
10+
* Called when the async processing starts respectively when the response arrives from the server. The passed
11+
* {@link Closeable} can be used to close/interrupt the processing.
1212
*/
1313
void onStart(Closeable closeable);
1414

‎src/main/java/com/github/dockerjava/core/async/ResultCallbackTemplate.java‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,8 @@ public abstract class ResultCallbackTemplate<RC_T extends ResultCallback<A_RES_T
2727

2828
private final static Logger LOGGER = LoggerFactory.getLogger(ResultCallbackTemplate.class);
2929

30+
private final CountDownLatch started = new CountDownLatch(1);
31+
3032
private final CountDownLatch completed = new CountDownLatch(1);
3133

3234
private Closeable stream;
@@ -39,6 +41,7 @@ public abstract class ResultCallbackTemplate<RC_T extends ResultCallback<A_RES_T
3941
public void onStart(Closeable stream) {
4042
this.stream = stream;
4143
this.closed = false;
44+
started.countDown();
4245
}
4346

4447
@Override
@@ -98,6 +101,27 @@ public RC_T awaitCompletion(long timeout, TimeUnit timeUnit) throws InterruptedE
98101
return (RC_T) this;
99102
}
100103

104+
/**
105+
* Blocks until {@link ResultCallback#onStart()} was called. {@link ResultCallback#onStart()} is called when the
106+
* request was processed on the server side and the response is incoming.
107+
*/
108+
@SuppressWarnings("unchecked")
109+
public RC_T awaitStarted() throws InterruptedException {
110+
started.await();
111+
return (RC_T) this;
112+
}
113+
114+
/**
115+
* Blocks until {@link ResultCallback#onStart()} was called or the given timeout occurs.
116+
* {@link ResultCallback#onStart()} is called when the request was processed on the server side and the response is
117+
* incoming.
118+
*/
119+
@SuppressWarnings("unchecked")
120+
public RC_T awaitStarted(long timeout, TimeUnit timeUnit) throws InterruptedException {
121+
started.await(timeout, timeUnit);
122+
return (RC_T) this;
123+
}
124+
101125
@CheckForNull
102126
protected RuntimeException getFirstError() {
103127
if (firstError != null) {

‎src/main/java/com/github/dockerjava/core/command/ExecStartResultCallback.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ public void onNext(Frame frame) {
5454
onError(e);
5555
}
5656

57+
LOGGER.debug(frame.toString());
5758
}
58-
LOGGER.debug(frame.toString());
5959
}
6060
}

‎src/main/java/com/github/dockerjava/netty/InvocationBuilder.java‎

Lines changed: 11 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -18,10 +18,11 @@
1818
import io.netty.handler.codec.json.JsonObjectDecoder;
1919
import io.netty.handler.stream.ChunkedStream;
2020
import io.netty.handler.stream.ChunkedWriteHandler;
21+
import io.netty.util.concurrent.Future;
22+
import io.netty.util.concurrent.GenericFutureListener;
2123

2224
import java.io.BufferedInputStream;
2325
import java.io.BufferedReader;
24-
import java.io.Closeable;
2526
import java.io.IOException;
2627
import java.io.InputStream;
2728
import java.io.InputStreamReader;
@@ -140,8 +141,6 @@ public <T> void get(TypeReference<T> typeReference, ResultCallback<T> resultCall
140141

141142
Channel channel = getChannel();
142143

143-
initCallback(channel, resultCallback);
144-
145144
JsonResponseCallbackHandler<T> jsonResponseHandler = new JsonResponseCallbackHandler<T>(typeReference,
146145
resultCallback);
147146

@@ -215,14 +214,22 @@ public InputStream post(final Object entity) {
215214
return callback.awaitResult();
216215
}
217216

218-
public void post(final Object entity, final InputStream stdin, ResultCallback<Frame> resultCallback) {
217+
public void post(final Object entity, final InputStream stdin, final ResultCallback<Frame> resultCallback) {
219218

220219
HttpRequestProvider requestProvider = httpPostRequestProvider(entity);
221220

222221
FramedResponseStreamHandler streamHandler = new FramedResponseStreamHandler(resultCallback);
223222

224223
final Channel channel = getChannel();
225224

225+
// result callback's close() method must be called when the servers closes the connection
226+
channel.closeFuture().addListener(new GenericFutureListener<Future<? super Void>>() {
227+
@Override
228+
public void operationComplete(Future<? super Void> future) throws Exception {
229+
resultCallback.onComplete();
230+
}
231+
});
232+
226233
HttpResponseHandler responseHandler = new HttpResponseHandler(requestProvider, resultCallback);
227234

228235
HttpConnectionHijackHandler hijackHandler = new HttpConnectionHijackHandler(responseHandler);
@@ -284,8 +291,6 @@ public <T> void post(final Object entity, TypeReference<T> typeReference, final
284291

285292
Channel channel = getChannel();
286293

287-
initCallback(channel, resultCallback);
288-
289294
JsonResponseCallbackHandler<T> jsonResponseHandler = new JsonResponseCallbackHandler<T>(typeReference,
290295
resultCallback);
291296

@@ -300,21 +305,6 @@ public <T> void post(final Object entity, TypeReference<T> typeReference, final
300305
return;
301306
}
302307

303-
private <T> void initCallback(final Channel channel, final ResultCallback<T> resultCallback) {
304-
Closeable closeable = new Closeable() {
305-
@Override
306-
public void close() throws IOException {
307-
try {
308-
channel.close().sync();
309-
} catch (InterruptedException e) {
310-
resultCallback.onError(e);
311-
}
312-
}
313-
};
314-
315-
resultCallback.onStart(closeable);
316-
}
317-
318308
private HttpRequest prepareDeleteRequest(String uri) {
319309

320310
FullHttpRequest request = new DefaultFullHttpRequest(HttpVersion.HTTP_1_1, HttpMethod.DELETE, uri);
@@ -406,8 +396,6 @@ public <T> void post(TypeReference<T> typeReference, ResultCallback<T> resultCal
406396

407397
Channel channel = getChannel();
408398

409-
initCallback(channel, resultCallback);
410-
411399
JsonResponseCallbackHandler<T> jsonResponseHandler = new JsonResponseCallbackHandler<T>(typeReference,
412400
resultCallback);
413401

@@ -442,8 +430,6 @@ public InputStream get() {
442430

443431
ResponseCallback<InputStream> resultCallback = new ResponseCallback<InputStream>();
444432

445-
initCallback(channel, resultCallback);
446-
447433
HttpResponseHandler responseHandler = new HttpResponseHandler(requestProvider, resultCallback);
448434

449435
HttpResponseStreamHandler streamHandler = new HttpResponseStreamHandler(resultCallback);

‎src/main/java/com/github/dockerjava/netty/handler/HttpResponseHandler.java‎

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
import io.netty.handler.codec.http.HttpResponse;
1212
import io.netty.handler.codec.http.LastHttpContent;
1313

14+
import java.io.Closeable;
1415
import java.nio.charset.Charset;
1516

1617
import com.github.dockerjava.api.async.ResultCallback;
@@ -24,8 +25,8 @@
2425
import com.github.dockerjava.api.exception.UnauthorizedException;
2526

2627
/**
27-
* Handler that is responsible to handle an incoming {@link HttpResponse}. It evaluates the status code and
28-
* triggers the appropriate lifecycle methods at the passed {@link ResultCallback}.
28+
* Handler that is responsible to handle an incoming {@link HttpResponse}. It evaluates the status code and triggers the
29+
* appropriate lifecycle methods at the passed {@link ResultCallback}.
2930
*
3031
* @author Marcus Linke
3132
*/
@@ -46,10 +47,18 @@ public HttpResponseHandler(HttpRequestProvider requestProvider, ResultCallback<?
4647
}
4748

4849
@Override
49-
protected void channelRead0(ChannelHandlerContext ctx, HttpObject msg) throws Exception {
50+
protected void channelRead0(final ChannelHandlerContext ctx, HttpObject msg) throws Exception {
5051
if (msg instanceof HttpResponse) {
5152

5253
response = (HttpResponse) msg;
54+
55+
resultCallback.onStart(new Closeable() {
56+
@Override
57+
public void close() {
58+
ctx.channel().close();
59+
}
60+
});
61+
5362
} else if (msg instanceof HttpContent) {
5463

5564
HttpContent content = (HttpContent) msg;

‎src/test/java/com/github/dockerjava/core/command/ExecStartCmdImplTest.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,7 @@ public void execStartAttached() throws Exception {
8181
ExecCreateCmdResponse execCreateCmdResponse = dockerClient.execCreateCmd(container.getId())
8282
.withAttachStdout(true).withCmd("touch", "/execStartTest.log").exec();
8383
dockerClient.execStartCmd(execCreateCmdResponse.getId()).withDetach(false).withTty(true)
84-
.exec(new ExecStartResultCallback(System.out, System.err));
84+
.exec(new ExecStartResultCallback(System.out, System.err)).awaitCompletion();
8585

8686
InputStream response = dockerClient.copyArchiveFromContainerCmd(container.getId(), "/execStartTest.log").exec();
8787
Boolean bytesAvailable = response.available() > 0;

‎src/test/java/com/github/dockerjava/core/command/InspectExecCmdImplTest.java‎

Lines changed: 17 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -57,33 +57,36 @@ public void inspectExec() throws Exception {
5757
dockerClient.startContainerCmd(container.getId()).exec();
5858

5959
// Check that file does not exist
60-
ExecCreateCmdResponse checkFileExec1 = dockerClient.execCreateCmd(container.getId())
61-
.withAttachStdout(true).withAttachStderr(true).withCmd("test", "-e", "/marker").exec();
60+
ExecCreateCmdResponse checkFileExec1 = dockerClient.execCreateCmd(container.getId()).withAttachStdout(true)
61+
.withAttachStderr(true).withCmd("test", "-e", "/marker").exec();
6262
LOG.info("Created exec {}", checkFileExec1.toString());
6363
assertThat(checkFileExec1.getId(), not(isEmptyString()));
64-
dockerClient.execStartCmd(checkFileExec1.getId()).exec(new ExecStartResultCallback(System.out, System.err)).awaitCompletion();
64+
dockerClient.execStartCmd(checkFileExec1.getId()).withDetach(false)
65+
.exec(new ExecStartResultCallback(System.out, System.err)).awaitCompletion();
6566
InspectExecResponse first = dockerClient.inspectExecCmd(checkFileExec1.getId()).exec();
67+
assertThat(first.isRunning(), is(false));
6668
assertThat(first.getExitCode(), is(1));
6769

6870
// Create the file
69-
ExecCreateCmdResponse touchFileExec = dockerClient.execCreateCmd(container.getId())
70-
.withAttachStdout(true).withAttachStderr(true).withCmd("touch", "/marker").exec();
71+
ExecCreateCmdResponse touchFileExec = dockerClient.execCreateCmd(container.getId()).withAttachStdout(true)
72+
.withAttachStderr(true).withCmd("touch", "/marker").exec();
7173
LOG.info("Created exec {}", touchFileExec.toString());
7274
assertThat(touchFileExec.getId(), not(isEmptyString()));
73-
dockerClient.execStartCmd(container.getId())
74-
.withExecId(touchFileExec.getId()).exec(new ExecStartResultCallback(System.out, System.err));
75+
dockerClient.execStartCmd(touchFileExec.getId()).withDetach(false)
76+
.exec(new ExecStartResultCallback(System.out, System.err)).awaitCompletion();
7577
InspectExecResponse second = dockerClient.inspectExecCmd(touchFileExec.getId()).exec();
78+
assertThat(second.isRunning(), is(false));
7679
assertThat(second.getExitCode(), is(0));
7780

78-
7981
// Check that file does exist now
80-
ExecCreateCmdResponse checkFileExec2 = dockerClient.execCreateCmd(container.getId())
81-
.withAttachStdout(true).withAttachStderr(true).withCmd("test", "-e", "/marker").exec();
82+
ExecCreateCmdResponse checkFileExec2 = dockerClient.execCreateCmd(container.getId()).withAttachStdout(true)
83+
.withAttachStderr(true).withCmd("test", "-e", "/marker").exec();
8284
LOG.info("Created exec {}", checkFileExec2.toString());
8385
assertThat(checkFileExec2.getId(), not(isEmptyString()));
84-
dockerClient.execStartCmd(container.getId())
85-
.withExecId(checkFileExec2.getId()).exec(new ExecStartResultCallback(System.out, System.err));
86+
dockerClient.execStartCmd(checkFileExec2.getId()).withDetach(false)
87+
.exec(new ExecStartResultCallback(System.out, System.err)).awaitCompletion();
8688
InspectExecResponse third = dockerClient.inspectExecCmd(checkFileExec2.getId()).exec();
89+
assertThat(third.isRunning(), is(false));
8790
assertThat(third.getExitCode(), is(0));
8891

8992
// Get container info and check its roundtrip to ensure the consistency
@@ -103,8 +106,8 @@ public void inspectExecNetworkSettings() throws IOException {
103106

104107
dockerClient.startContainerCmd(container.getId()).exec();
105108

106-
ExecCreateCmdResponse exec = dockerClient.execCreateCmd(container.getId())
107-
.withAttachStdout(true).withAttachStderr(true).withCmd("/bin/bash").exec();
109+
ExecCreateCmdResponse exec = dockerClient.execCreateCmd(container.getId()).withAttachStdout(true)
110+
.withAttachStderr(true).withCmd("/bin/bash").exec();
108111
LOG.info("Created exec {}", exec.toString());
109112
assertThat(exec.getId(), not(isEmptyString()));
110113

‎src/test/java/com/github/dockerjava/netty/exec/ExecStartCmdExecTest.java‎

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -58,11 +58,8 @@ public void execStart() throws Exception {
5858

5959
ExecCreateCmdResponse execCreateCmdResponse = dockerClient.execCreateCmd(container.getId())
6060
.withAttachStdout(true).withCmd("touch", "/execStartTest.log").exec();
61-
dockerClient.execStartCmd(execCreateCmdResponse.getId()).exec(
62-
new ExecStartResultCallback(System.out, System.err));
63-
64-
LOG.info("Wait for 5 seconds");
65-
Thread.sleep(5000);
61+
dockerClient.execStartCmd(execCreateCmdResponse.getId()).withDetach(false)
62+
.exec(new ExecStartResultCallback(System.out, System.err)).awaitCompletion();
6663

6764
InputStream response = dockerClient.copyArchiveFromContainerCmd(container.getId(), "/execStartTest.log").exec();
6865

@@ -88,8 +85,8 @@ public void execStartAttached() throws Exception {
8885

8986
ExecCreateCmdResponse execCreateCmdResponse = dockerClient.execCreateCmd(container.getId())
9087
.withAttachStdout(true).withCmd("touch", "/execStartTest.log").exec();
91-
dockerClient.execStartCmd(execCreateCmdResponse.getId()).withDetach(false).withTty(true)
92-
.exec(new ExecStartResultCallback(System.out, System.err));
88+
dockerClient.execStartCmd(execCreateCmdResponse.getId()).withDetach(false)
89+
.exec(new ExecStartResultCallback(System.out, System.err)).awaitCompletion();
9390

9491
InputStream response = dockerClient.copyArchiveFromContainerCmd(container.getId(), "/execStartTest.log").exec();
9592
Boolean bytesAvailable = response.available() > 0;
@@ -141,7 +138,7 @@ public void execStartNotAttachedStdin() throws Exception {
141138

142139
ExecCreateCmdResponse execCreateCmdResponse = dockerClient.execCreateCmd(container.getId())
143140
.withAttachStdout(true).withAttachStdin(false).withCmd("/bin/sh").exec();
144-
dockerClient.execStartCmd(execCreateCmdResponse.getId()).withDetach(false).withTty(true).withStdIn(stdin)
141+
dockerClient.execStartCmd(execCreateCmdResponse.getId()).withDetach(false).withStdIn(stdin)
145142
.exec(new ExecStartResultCallback(stdout, System.err)).awaitCompletion(5, TimeUnit.SECONDS);
146143

147144
assertEquals(stdout.toString(), "");

‎src/test/java/com/github/dockerjava/netty/exec/InspectExecCmdExecTest.java‎

Lines changed: 17 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -59,33 +59,36 @@ public void inspectExec() throws Exception {
5959
dockerClient.startContainerCmd(container.getId()).exec();
6060

6161
// Check that file does not exist
62-
ExecCreateCmdResponse checkFileExec1 = dockerClient.execCreateCmd(container.getId())
63-
.withAttachStdout(false).withAttachStderr(false).withCmd("test", "-e", "/marker").exec();
62+
ExecCreateCmdResponse checkFileExec1 = dockerClient.execCreateCmd(container.getId()).withAttachStdout(true)
63+
.withAttachStderr(true).withCmd("test", "-e", "/marker").exec();
6464
LOG.info("Created exec {}", checkFileExec1.toString());
6565
assertThat(checkFileExec1.getId(), not(isEmptyString()));
66-
dockerClient.execStartCmd(checkFileExec1.getId()).exec(new ExecStartResultCallback(System.out, System.err));
66+
dockerClient.execStartCmd(checkFileExec1.getId()).withDetach(false)
67+
.exec(new ExecStartResultCallback(System.out, System.err)).awaitCompletion();
6768
InspectExecResponse first = dockerClient.inspectExecCmd(checkFileExec1.getId()).exec();
69+
assertThat(first.isRunning(), is(false));
6870
assertThat(first.getExitCode(), is(1));
6971

7072
// Create the file
71-
ExecCreateCmdResponse touchFileExec = dockerClient.execCreateCmd(container.getId())
72-
.withAttachStdout(true).withAttachStderr(true).withCmd("touch", "/marker").exec();
73+
ExecCreateCmdResponse touchFileExec = dockerClient.execCreateCmd(container.getId()).withAttachStdout(true)
74+
.withAttachStderr(true).withCmd("touch", "/marker").exec();
7375
LOG.info("Created exec {}", touchFileExec.toString());
7476
assertThat(touchFileExec.getId(), not(isEmptyString()));
75-
dockerClient.execStartCmd(container.getId())
76-
.withExecId(touchFileExec.getId()).exec(new ExecStartResultCallback(System.out, System.err));
77+
dockerClient.execStartCmd(touchFileExec.getId()).withDetach(false)
78+
.exec(new ExecStartResultCallback(System.out, System.err)).awaitCompletion();
7779
InspectExecResponse second = dockerClient.inspectExecCmd(touchFileExec.getId()).exec();
80+
assertThat(second.isRunning(), is(false));
7881
assertThat(second.getExitCode(), is(0));
7982

80-
8183
// Check that file does exist now
82-
ExecCreateCmdResponse checkFileExec2 = dockerClient.execCreateCmd(container.getId())
83-
.withAttachStdout(true).withAttachStderr(true).withCmd("test", "-e", "/marker").exec();
84+
ExecCreateCmdResponse checkFileExec2 = dockerClient.execCreateCmd(container.getId()).withAttachStdout(true)
85+
.withAttachStderr(true).withCmd("test", "-e", "/marker").exec();
8486
LOG.info("Created exec {}", checkFileExec2.toString());
8587
assertThat(checkFileExec2.getId(), not(isEmptyString()));
86-
dockerClient.execStartCmd(container.getId())
87-
.withExecId(checkFileExec2.getId()).exec(new ExecStartResultCallback(System.out, System.err));
88+
dockerClient.execStartCmd(checkFileExec2.getId()).withDetach(false)
89+
.exec(new ExecStartResultCallback(System.out, System.err)).awaitCompletion();
8890
InspectExecResponse third = dockerClient.inspectExecCmd(checkFileExec2.getId()).exec();
91+
assertThat(third.isRunning(), is(false));
8992
assertThat(third.getExitCode(), is(0));
9093

9194
// Get container info and check its roundtrip to ensure the consistency
@@ -105,8 +108,8 @@ public void inspectExecNetworkSettings() throws IOException {
105108

106109
dockerClient.startContainerCmd(container.getId()).exec();
107110

108-
ExecCreateCmdResponse exec = dockerClient.execCreateCmd(container.getId())
109-
.withAttachStdout(true).withAttachStderr(true).withCmd("/bin/bash").exec();
111+
ExecCreateCmdResponse exec = dockerClient.execCreateCmd(container.getId()).withAttachStdout(true)
112+
.withAttachStderr(true).withCmd("/bin/bash").exec();
110113
LOG.info("Created exec {}", exec.toString());
111114
assertThat(exec.getId(), not(isEmptyString()));
112115

0 commit comments

Comments
 (0)