-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Fixes #1448 #1449
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fixes #1448 #1449
Conversation
@Test | ||
public void closeStdinWithStdinOnce() throws Exception { | ||
Assume.assumeTrue("supports stdin attach", getFactoryType().supportsStdinAttach()); | ||
Assume.assumeFalse("not fixed for Apache HttpClient 5.0", getFactoryType().equals(FactoryType.HTTPCLIENT5)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is it a WIP or you was unable to fix it for AHC5?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I addressed this problem because my colleague at JetBrains stuck into the situation where our Python helper script misbehaved. And we use only OkHttp implementation.
However I dived into AHC5 implementation. It has pretty similar UnixDomainSocket
class. But fixing only this class does not help because org.apache.hc.core5.http.impl.io.ContentLengthOutputStream.close()
does not call close()
method on underlying outputStream
(which belongs to UnixDomainSocket
that could be fixed) and, unfortunately,ContentLengthOutputStream
class is situated in org.apache.httpcomponents.core5:httpcore5:5.0
artifact.
Here is the stacktrace where magic does not happen:
"docker-java-httpclient5-hijacking-stream-1739943297@6989" daemon prio=5 tid=0x3d nid=NA runnable
java.lang.Thread.State: RUNNABLE
at org.apache.hc.core5.http.impl.io.ContentLengthOutputStream.close(ContentLengthOutputStream.java:92)
at org.apache.hc.core5.http.impl.io.DefaultBHttpClientConnection.sendRequestEntity(DefaultBHttpClientConnection.java:154)
at com.github.dockerjava.httpclient5.HijackingHttpRequestExecutor.lambda$executeHijacked$0(HijackingHttpRequestExecutor.java:82)
at com.github.dockerjava.httpclient5.HijackingHttpRequestExecutor$$Lambda$233.1769119455.run(Unknown Source:-1)
at java.lang.Thread.run(Thread.java:834)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have investigated the problem in AHC5 implementation. Although the org.apache.hc.core5.http.impl.io.ContentLengthOutputStream.close()
method does not close the underlying OutputStream
, it is possible to shut down the OutputStream
in com.github.dockerjava.httpclient5.HijackingHttpRequestExecutor
class right after conn.sendRequestEntity(fakeRequest);
call:
if (conn instanceof ManagedHttpClientConnection) {
((ManagedHttpClientConnection) conn).getSocket().shutdownOutput();
}
However, for that to work, similar changes to the UnixDomainSocket
class (under docker-java-transport-httpclient5) are needed because, by default, UnixDomainSocket.shutdownOutput()
does nothing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@@tejksat gentle ping :)
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
Superseeded by #1567 |
Fixes #1448