Preserve the order of requests to Docker daemon using DefaultInvocationBuilder (#1492)#1494
Conversation
| thread.start(); | ||
|
|
||
| try { | ||
| onStart.await(); |
There was a problem hiding this comment.
callback's onStart should be used to detect started operation. Blocking here is an unexpected behaviour change
There was a problem hiding this comment.
also see awaitStarted on ResultCallbackTemplate
There was a problem hiding this comment.
Sorry for the possible confusion: onStart is CountDownLatch here actually. I will rename it.
There was a problem hiding this comment.
my point is that DefaultInvocationBuilder should not be changed. Instead, the callee should use awaitStarted() or similar means
There was a problem hiding this comment.
Thank you, I got it now. I thought there will be inconsistency with other post and get methods of DefaultInvocationBuilder which do not accept ResultCallback as a parameter. The requests in these cases are executed in the same thread and awaitStarted() is not required. But this is the expected behavior?
There was a problem hiding this comment.
Those that have no ResultCallback as a parameter are blocking methods, while ResultCallback-based ones are kinda asynchronous, hence no blocking behaviour
`attachContainerStdinUnsupported()` test requires neither starting the container nor waiting on `awaitStarted()` of attach's callback.
docker-java/src/test/java/com/github/dockerjava/cmd/AttachContainerCmdIT.java
Show resolved
Hide resolved
Restore starting the container in the test.
2 participants
The problem is that using
awaitStarted()onattach's callback on a non-running container leads to the infinite wait for Jersey transport implementation.This happens when
SelectiveLoggingFilteris used inDefaultInvocationBuilderwhere it is created withprintEntity = trueparameter. In this case the implementation ofClientResponseFilter.filter(...)method in the baseLoggingFilterclass blocks atresponseContext.hasEntity()until the data is available in the response. This happens becausehasEntity()reads a single byte to check whether there is any data available in the stream. In the case ofattachcommand this method blocks until there is data in stdout/stderr streams. Having data from the streams requires container running, which we cannot do safely withoutawaitStarted()becauseattachcommand is performed in a separate thread and we need to runstartonly afterattachrequest is executed not to loose the data from stdout/stderr streams.