Exec start command: detect end of STDIN stream by rtimush · Pull Request #472 · docker-java/docker-java

@rtimush

Fix for #471.

The fix itself is to close the write side of the socket to indicate the end of stream.
shutdownOutput is available for unix domain sockets since netty 4.1.0.CR3.

@marcuslinke

Hmm. Don't know if we can use EpollSocketChannel instead of EpollDomainSocketChannel. This one has a shutdownOutput() method at least...

@marcuslinke

@rtimush

I don't think we can use EpollSocketChannel for unix domain sockets.

@rtimush

Netty devs confirmed that it's ok to add shutdownOutput to EpollDomainSocketChannel.

@marcuslinke

@rtimush So lets wait for a new netty release before merge this. Good job! Thanks.

@rtimush

@rtimush

Updated to netty 4.1.0.CR3. ExecStartCmdExecTest should now pass both with tcp and unix protocols, but I tested tcp only.

@codecov-io

Current coverage is 19.02%

Merging #472 into master will not affect coverage as of 166a8fc

@@            master    #472   diff @@
======================================
  Files          281     281       
  Stmts         5562    5561     -1
  Branches       524     524       
  Methods          0       0       
======================================
  Hit           1058    1058       
  Partial         82      82       
+ Missed        4422    4421     -1

Review entire Coverage Diff as of 166a8fc

Powered by Codecov. Updated on successful CI builds.

marcuslinke


// we close the writing side of the socket, but keep the read side open to transfer stdout/stderr
// unfortunately, shutdownOutput is not supported by io.netty.channel.epoll.EpollDomainSocketChannel
if (channel instanceof DuplexChannel) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe better throw a RuntimeException when not a DuplexChannel here?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, I can change channel type to DuplexChannel because it extends Channel and avoid type checks. Will have to cast in InetSocketInitializer and UnixDomainSocketInitializer.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please see the last commit

@marcuslinke

@rtimush OK, tested with unix socket successfully now so lets merge.

marcuslinke added a commit that referenced this pull request

Feb 24, 2016
Exec start command: detect end of STDIN stream