Fix issue with jersey and unix domain sockets by marcuslinke · Pull Request #697 · docker-java/docker-java

marcuslinke · 2016-09-07T22:20:25Z

This eliminates the socket already closed exception (org.newsclub.net.unix.AFUNIXSocketException: Bad file descriptor). The problem occurs when the underlying connection of a stream is closed and chunked encoding is active. The apache ChunkedInputStream still tries to read till the end of the current chunk while close()ing itself. This is not possible as the connection/socket is already closed.

Sadly it needs to fix two classes AFUNIXSocketImpl and ChunkedInputStream to solve the issue.

This change is

marcuslinke · 2016-09-08T05:15:19Z

pom.xml

+			<artifactId>junixsocket-native-common</artifactId>
+			<version>${junixsocket.version}</version>
+		</dependency>
+		<dependency>


Dependency to jnr-unixsocket must be removed...

KostyaSha · 2016-09-09T11:38:34Z

pom.xml

-<!-- 			</extension> -->
-<!-- 		</extensions> -->
+		<!-- <extensions> -->
+		<!-- <extension> -->


Makes sense 👍

KostyaSha · 2016-09-09T11:40:11Z

👍

marcuslinke · 2016-09-09T12:18:43Z

@KostyaSha What is the difference between the two travis builds continuous-integration/travis-ci/pushand continuous-integration/travis-ci/pr ?

KostyaSha · 2016-09-09T12:38:07Z

Obviously pull request and branch build ;)

KostyaSha · 2016-09-09T12:39:08Z

So if you don't plan cooperate on branch, then do branch in your fork and open PR.

KostyaSha · 2016-09-09T13:06:08Z

(com.github.dockerjava.core.command.EventsCmdImplTest)  Time elapsed: 29.805 sec  <<< FAILURE!
java.lang.AssertionError: Received only: [] expected [true] but found [false]
    at com.github.dockerjava.core.command.EventsCmdImplTest.testEventStreamingWithFilter(EventsCmdImplTest.java:122)
Results :
Failed tests: 
com.github.dockerjava.core.command.EventsCmdImplTest.(com.github.dockerjava.core.command.EventsCmdImplTest)
  Run 1: PASS
  Run 2: PASS
  Run 3: PASS
  Run 4: EventsCmdImplTest.testEventStreamingWithFilter:122->Assert.assertTrue:42->Assert.failNotEquals:513->Assert.fail:94 Received only: [] expected [true] but found [false]

retriggering build

codecov-io · 2016-09-09T13:19:57Z

Current coverage is 71.09% (diff: 47.03%)

Merging #697 into master will decrease coverage by 0.20%

@@             master       #697   diff @@
==========================================
  Files           300        302     +2   
  Lines          6202       6484   +282   
  Methods           0          0          
  Messages          0          0          
  Branches        528        585    +57   
==========================================
+ Hits           4422       4610   +188   
- Misses         1531       1594    +63   
- Partials        249        280    +31

Powered by Codecov. Last update 74d506d...066f177

KostyaSha · 2016-09-09T13:21:47Z

Also place somewhere in javadoc why clases was copied.

KostyaSha · 2016-09-13T22:15:20Z

@marcuslinke please set milestones before merge and update changelog after merge

KostyaSha · 2016-09-13T23:00:44Z

[WARNING] docker-java-3.0.6.jar, httpcore-4.4.5.jar define 1 overlapping classes: 
[WARNING]   - org.apache.http.impl.io.ChunkedInputStream
[WARNING] docker-java-3.0.6.jar, junixsocket-common-2.0.4.jar define 5 overlapping classes: 
[WARNING]   - org.newsclub.net.unix.AFUNIXSocketImpl$AFUNIXOutputStream
[WARNING]   - org.newsclub.net.unix.AFUNIXSocketImpl
[WARNING]   - org.newsclub.net.unix.AFUNIXSocketImpl$AFUNIXInputStream
[WARNING]   - org.newsclub.net.unix.AFUNIXSocketImpl$Lenient
[WARNING]   - org.newsclub.net.unix.AFUNIXSocketImpl$1

KostyaSha · 2016-09-13T23:04:25Z

@marcuslinke where ChunkedInputStream is used? or do you trying replace classes? Classes are messed..

KostyaSha · 2016-09-14T00:30:49Z

On mac after shading i have

Caused by: java.lang.UnsatisfiedLinkError: com.github.kostyasha.yad_docker_java.org.newsclub.net.unix.NativeUnixSocket.connect(Ljava/lang/String;Ljava/io/FileDescriptor;)V
    at com.github.kostyasha.yad_docker_java.org.newsclub.net.unix.NativeUnixSocket.connect(Native Method)
    at com.github.kostyasha.yad_docker_java.org.newsclub.net.unix.AFUNIXSocketImpl.connect(AFUNIXSocketImpl.java:137)
    at com.github.kostyasha.yad_docker_java.org.newsclub.net.unix.AFUNIXSocket.connect(AFUNIXSocket.java:107)
    at com.github.kostyasha.yad_docker_java.com.github.dockerjava.jaxrs.ApacheUnixSocket.connect(ApacheUnixSocket.java:62)
    at com.github.kostyasha.yad_docker_java.com.github.dockerjava.jaxrs.UnixConnectionSocketFactory.connectSocket(UnixConnectionSocketFactory.java:74)
    at com.github.kostyasha.yad_docker_java.org.apache.http.impl.conn.DefaultHttpClientConnectionOperator.connect(DefaultHttpClientConnectionOperator.java:134)
    at com.github.kostyasha.yad_docker_java.org.apache.http.impl.conn.PoolingHttpClientConnectionManager.connect(PoolingHttpClientConnectionManager.java:353)
    at com.github.kostyasha.yad_docker_java.org.apache.http.impl.execchain.MainClientExec.establishRoute(MainClientExec.java:380)
    at com.github.kostyasha.yad_docker_java.org.apache.http.impl.execchain.MainClientExec.execute(MainClientExec.java:236)
    at com.github.kostyasha.yad_docker_java.org.apache.http.impl.execchain.ProtocolExec.execute(ProtocolExec.java:184)
    at com.github.kostyasha.yad_docker_java.org.apache.http.impl.execchain.RetryExec.execute(RetryExec.java:88)
    at com.github.kostyasha.yad_docker_java.org.apache.http.impl.execchain.RedirectExec.execute(RedirectExec.java:110)
    at com.github.kostyasha.yad_docker_java.org.apache.http.impl.client.InternalHttpClient.doExecute(InternalHttpClient.java:184)
    at com.github.kostyasha.yad_docker_java.org.apache.http.impl.client.CloseableHttpClient.execute(CloseableHttpClient.java:71)
    at com.github.kostyasha.yad_docker_java.org.glassfish.jersey.apache.connector.ApacheConnector.apply(ApacheConnector.java:435)
    at com.github.kostyasha.yad_docker_java.org.glassfish.jersey.client.ClientRuntime.invoke(ClientRuntime.java:252)
    ... 81 more

Will try compare with linux env and plain docker-java.

KostyaSha · 2016-09-14T00:50:06Z

M... it impossible relocate java class because it depends on native library?

marcuslinke · 2016-09-14T05:34:51Z

@KostyaSha Yes, I've replaced two classes as described in the first comment. Didn't know that this is a problem with shading... :(

KostyaSha · 2016-09-14T09:55:37Z

Well, it warning, but having conflicting package-class is questionable.

marcuslinke · 2016-09-14T10:02:37Z

Yes, thats ugly but fixing this in upstream projects (apache HTTPCore AND junixsockets) would take ages I think.

papegaaij · 2016-09-20T09:10:47Z

You cannot simply copy a class an expect it to shadow a class from a dependency. This all depends on classloading order, which can be non-deterministic in some cases (such as packaging multiple jars in a war or ear). Your only options are to get this fixed upstream or to work around this some other way. The currently solution breaks our build because the maven-enforcer-plugin detects duplicate classes.

alexburchak · 2017-10-03T10:09:08Z

My project became a victim of the class loading order issue :)
ChunkedInputStream in docker-java (from the commit), which has that try/catch fix, is not loaded and original class implementation from httpcore is used instead.
Exception thrown in close() method (after invoking read()) is not caught and thus propagated above.
Even worse, the code is catching one of ConnectionClosedException or TruncatedChunkException, but instead of the last one, there should have been MalformedChunkCodingException (it's parent).

and because we no longer need our custom version of AFUNIXSocketImpl from docker-java#697

marcuslinke reviewed Sep 8, 2016
View reviewed changes

marcuslinke force-pushed the fix-jersey-unixsocket branch from d410911 to 066f177 Compare September 8, 2016 20:30

KostyaSha reviewed Sep 9, 2016
View reviewed changes

marcuslinke force-pushed the fix-jersey-unixsocket branch 2 times, most recently from ea6bdbc to e51b898 Compare September 9, 2016 21:13

Fix issue with jersey and unix domain sockets

d6610ab

marcuslinke force-pushed the fix-jersey-unixsocket branch from e51b898 to d6610ab Compare September 10, 2016 15:28

marcuslinke merged commit d39d7f2 into master Sep 10, 2016

marcuslinke deleted the fix-jersey-unixsocket branch September 10, 2016 16:54

marcuslinke mentioned this pull request Sep 12, 2016

Fix dependencies to support Docker for OSX. #677

Closed

tejksat pushed a commit to tejksat/docker-java that referenced this pull request Sep 13, 2016

Fix issue with jersey and unix domain sockets (docker-java#697)

3fdaad2

KostyaSha added this to the 3.0.6 milestone Sep 13, 2016

KostyaSha added the type/bug label Sep 13, 2016

CodingFabian mentioned this pull request Jan 5, 2017

Patch from docker-java for reading from closed socket kohlschutter/junixsocket#31

Closed

panuse pushed a commit to TuKangTech/docker-java that referenced this pull request Aug 20, 2017

Fix issue with jersey and unix domain sockets (docker-java#697)

a79014d

dbyron0 pushed a commit to locationlabs/docker-java that referenced this pull request Jan 2, 2019

use version 2.1.2 of junixsocket to stay up to date

3f163f0

and because we no longer need our custom version of AFUNIXSocketImpl from docker-java#697

Conversation

marcuslinke commented Sep 7, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

marcuslinke Sep 8, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

KostyaSha Sep 9, 2016

Choose a reason for hiding this comment

Uh oh!

marcuslinke Sep 9, 2016

Choose a reason for hiding this comment

Uh oh!

KostyaSha commented Sep 9, 2016

Uh oh!

marcuslinke commented Sep 9, 2016

Uh oh!

KostyaSha commented Sep 9, 2016

Uh oh!

KostyaSha commented Sep 9, 2016

Uh oh!

KostyaSha commented Sep 9, 2016

Uh oh!

codecov-io commented Sep 9, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Current coverage is 71.09% (diff: 47.03%)

Uh oh!

KostyaSha commented Sep 9, 2016

Uh oh!

KostyaSha commented Sep 13, 2016

Uh oh!

KostyaSha commented Sep 13, 2016

Uh oh!

KostyaSha commented Sep 13, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

KostyaSha commented Sep 14, 2016

Uh oh!

KostyaSha commented Sep 14, 2016

Uh oh!

marcuslinke commented Sep 14, 2016

Uh oh!

KostyaSha commented Sep 14, 2016

Uh oh!

marcuslinke commented Sep 14, 2016

Uh oh!

papegaaij commented Sep 20, 2016

Uh oh!

alexburchak commented Oct 3, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

marcuslinke commented Sep 7, 2016 •

edited

Loading

marcuslinke Sep 8, 2016 •

edited

Loading

codecov-io commented Sep 9, 2016 •

edited

Loading

KostyaSha commented Sep 13, 2016 •

edited

Loading

alexburchak commented Oct 3, 2017 •

edited

Loading