Extend abstract Java socket classes

[monsanto]

It's difficult to use this library in conjunction with the rest of the Java ecosystem because the standard abstract socket classes are not implemented. Was there a specific reason this isn't done? In other words, if I tried to do this myself, is there something non-obvious that would block me?

For reference, http://code.google.com/p/junixsocket/ supports the non-NIO socket interfaces.

[softprops]

• 1

[softprops]

That was meant to be a plus one

[monsanto]

Cleaning up my issues, also I don't think the project maintainers care

[softprops]

For what its worth I gave this a shot. It was kind of an ugly hack but it worked for what I needed. https://github.com/softprops/unisockets

[headius]

Wow, I never even noticed people had filed issues here. Reopening...seems like a good idea.

[headius]

@softprops Do you think you could contribute your socket impls back to this project? If I were to start implementing it myself right now it probably would just come out like yours.

[headius]

@monsanto We definitely do care...I apologize for not seeing this issue for so long!

[headius]

One more ping to see if there's user interest in getting this done...

[gesellix]

@headius I guess there is some interest :-)

[headius]

@gesellix Well, perhaps we can duplicate what @softprops did?

[headius]

👍 Let's do it for 0.14 finally. Help?

[gesellix]

@headius I guess I won't be of any help, but I know that the folks at https://github.com/docker-java/docker-java are interested in a solution. Maybe they can help testing?

[headius]

@gesellix That sounds great! Do you know anyone we can @ to get them in here?

[gesellix]

@headius the driving force behind Docker-Java are @marcuslinke and @KostyaSha. @normanmaurer also makes sense to be included in the conversation.

To give you a bit more context: Docker-Java wants to support Unix sockets via Netty, which doesn't seem to be easy on OS X. Related issues are:

• docker for mac compatibility  docker-java/docker-java#537 (comment)
• Add kqueue channel implementation  netty/netty#2448 (comment)

Hope that helps to improve all those libraries :-)

[marcuslinke]

Hi, I'm not really good in this NIO stuff... However, I tried to adopt @softprops code to make it work with netty in the context of docker-java (https://github.com/docker-java/docker-java/blob/netty-unixsocket-experiment)

I've modified jnr.enxio.channels.NativeSocketChannel to inherit java.nio.channels.SocketChannel and modified jnr.unixsocket.UnixSocketChannel to extend it:

https://github.com/docker-java/docker-java/blob/netty-unixsocket-experiment/src/main/java/jnr/unixsocket/UnixSocketChannel.java

https://github.com/docker-java/docker-java/blob/netty-unixsocket-experiment/src/main/java/jnr/enxio/channels/NativeSocketChannel.java

Maybe this is useful for somebody and should be integrated into the jnr project somehow?

[headius]

@marcuslinke I'd love to see PRs for both of those!

[marcuslinke]

@headius Should I rename these classes in the PR or should both replace the existing implementations? Sorry for asking but I don't fully understand the whole NIO class hierarchy and its detailed meaning.

[headius]

Actually it occurs to me these might not be the right classes...I'll have a look into this myself using your fork as a guide.

[headius]

Ok, I started looking into it, and this is a bit of a challenge to do right.

First off, extending SocketChannel brings in a lot of methods that don't make sense with the way jnr-enxio is structures. Specifically, enxio expects you to pass in an already-connected socket fd, so bind, connect, isConnected and so on don't make sense. This also means that constructors and methods that take a SocketAddress have no meaning, because NativeSocketChannel currently has no knowledge of what the socket's address is, nor can it change it by rebinding or reconnecting.

Second, some of the SocketChannel methods that we are forced to implement only exist in Java 7 and use Java 7+ classes. It might be ok if we don't mark them @Override, or Java 6 JVMs might still refuse to load. We might also be safe dropping Java 6 support at this point.

If we can address these items, then UNIXSocket will become a SocketChannel just by merit of extending the enxio classes. I'm just worried about claiming it's a SocketChannel when half the SocketChannel methods don't work.

[headius]

Here's what I came up with, similar to @marcuslinke's version: https://gist.github.com/headius/8eba1c78feeea8c76b0ae624839eb7a6

[KostyaSha]

. We might also be safe dropping Java 6 support at this point.

👍 everybody deal with it enough!

[marcuslinke]

@headius In my original code the NativeSocketChannel class was changed to be abstract so an extendig class has to implement those missing methods from SocketChannel. UnixSocketChannel does so - at least it provides the connection logic. I think NativeSocketChannel must be abstract orUnixSocketChannel should inherit from SocketChannel directly.

[headius]

I worry about making NativeSocketChannel abstract, since it breaks a public API. Right now if you want a native socket behavior in NIO you can just stuff any old fd into this. If it's abstract, we'd at least need to provide a non-abstract one that accepts an fd, in which case we'd have to stub out all these same methods anyway.

So at the very least NativeSocketChannel needs to remain concrete, but perhaps it and UNIXSocketChannel could extend the same abstract base that extends SocketChannel?

[headius]

Re Java 6: JRuby 1.7.x still supports Java 6 and uses jnr-enxio. JRuby 1.7.x will be EOL around the end of this year.

We could freeze it at an old version, but that's obviously problematic if we need to update enxio before EOL.

[marcuslinke]

So at the very least NativeSocketChannel needs to remain concrete, but perhaps it and UNIXSocketChannel could extend the same abstract base that extends SocketChannel?

@headius I think we should leave NativeSocketChannel as is and introduce a new AbstractNativeSocketChannel class that extends SocketChannel. This way NativeSocketChannel (which in fact is a "NativeSelectableChannel" and should be renamed so) haven't implement all those useless methods from SocketChannel. WDYT?

[headius]

@marcuslinke Yeah that's kinda how I'm leaning too. The way NativeSocketChannel works doesn't fit very well into SocketChannel due to it just wrapping an already-connected socket file descriptor.

[marcuslinke]

Started PR here: jnr/jnr-enxio#20

@headius What about UnixSocketChannel? I guess the new implementation should replace the existing one, shouldn't it?

[headius]

@marcuslinke Yes, we'd probably want to have UnixSocketChannel extend AbstractNativeSocketChannel, right? NativeSocketChannel would remain a raw fd wrapper for folks that need it, but other socket types (like UnixSocketChannel) would hide that bit behind SocketAddress constructors, etc. You'd still be able to get the fd out and probably pass one directly in, but the preferred way would be SocketAddress-aware subclasses.

[marcuslinke]

OK. I've created another PR for the unix socket stuff: #33

[CodingFabian]

Just wanted to chime in,
We are actually happy users of http://code.google.com/p/junixsocket/ but from now and then we have some strange issues with the library. In fact we also use it for docker socket reading and we have had customers where the socket just stayed open leaking native memory all over the place.

Reproducing those issues is close to impossible, and I intended to try jnr-unixsocket, but only having it return a "Socket" would be a drop in replacement for us.
Java8 compatibility would be sufficient here :)

[CodingFabian]

oh it looks like this was released already :-) going to try it out

[swankjesse]

Worth taking this a few steps further, and implementing the TCP interfaces of SocketFactory, ServerSocket, and ServerSocketFactory?

I’ve got the basics here:
https://github.com/square/okhttp/blob/master/samples/unixdomainsockets/src/main/java/okhttp3/unixdomainsockets/UnixDomainServerSocketFactory.java

[headius]

I believe this was resolved in #35 and released in 0.15.

If there's additional work to be done let's open a new issue (this one is quite old) and we can discuss there.