Skip to content

ARROW-6461: [Java] Prevent EchoServer from closing the client socket after writing#5288

Closed
BryanCutler wants to merge 1 commit intoapache:masterfrom
BryanCutler:java-EchoServer-prevent-close-6461
Closed

ARROW-6461: [Java] Prevent EchoServer from closing the client socket after writing#5288
BryanCutler wants to merge 1 commit intoapache:masterfrom
BryanCutler:java-EchoServer-prevent-close-6461

Conversation

@BryanCutler
Copy link
Copy Markdown
Member

This change prevents a race condition where the EchoServer closes the client socket immediately after writing batches and the client may have not finished reading from the socket. The socket was being closed in three places: ArrowStreamReader.close() and ArrowStreamWriter.close() when being AutoClosed, and when the ClientConnection went out of scope. The race condition was prevalent in #5229 and consitently failing the EchoServerTests.basicTest.

@BryanCutler
Copy link
Copy Markdown
Member Author

@tianchen92 and @emkornfield , this fixed the error for me although I could only recreate the issue with the changes in #5229 with the 8 byte prefix on (by default). If you want you can try it out in your branch and see if we pass tests first, or if this looks ok we could merge this and then rebase your branch.

Copy link
Copy Markdown
Contributor

@tianchen92 tianchen92 left a comment

Choose a reason for hiding this comment

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

Thanks @BryanCutler , I think we could get this merged once build pass, and I could rebase on my branch later.

@codecov-io
Copy link
Copy Markdown

Codecov Report

❗ No coverage uploaded for pull request base (master@314e9f0). Click here to learn what that means.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##             master    #5288   +/-   ##
=========================================
  Coverage          ?   89.71%           
=========================================
  Files             ?      693           
  Lines             ?   104695           
  Branches          ?        0           
=========================================
  Hits              ?    93926           
  Misses            ?    10769           
  Partials          ?        0

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 314e9f0...3763cbc. Read the comment docs.

@BryanCutler
Copy link
Copy Markdown
Member Author

merged to master

@BryanCutler BryanCutler deleted the java-EchoServer-prevent-close-6461 branch September 5, 2019 17:45
tianchen92 pushed a commit to tianchen92/arrow that referenced this pull request Sep 6, 2019
…after writing

This change prevents a race condition where the `EchoServer` closes the client socket immediately after writing batches and the client may have not finished reading from the socket. The socket was being closed in three places: `ArrowStreamReader.close()` and `ArrowStreamWriter.close()` when being AutoClosed, and when the `ClientConnection` went out of scope. The race condition was prevalent in apache#5229 and consitently failing the `EchoServerTests.basicTest`.

Closes apache#5288 from BryanCutler/java-EchoServer-prevent-close-6461 and squashes the following commits:

3763cbc <Bryan Cutler> Prevent EchoServer from closing the client socket after writing batches

Authored-by: Bryan Cutler <cutlerb@gmail.com>
Signed-off-by: Bryan Cutler <cutlerb@gmail.com>
pribor pushed a commit to GlobalWebIndex/arrow that referenced this pull request Oct 24, 2025
…after writing

This change prevents a race condition where the `EchoServer` closes the client socket immediately after writing batches and the client may have not finished reading from the socket. The socket was being closed in three places: `ArrowStreamReader.close()` and `ArrowStreamWriter.close()` when being AutoClosed, and when the `ClientConnection` went out of scope. The race condition was prevalent in apache#5229 and consitently failing the `EchoServerTests.basicTest`.

Closes apache#5288 from BryanCutler/java-EchoServer-prevent-close-6461 and squashes the following commits:

3763cbc <Bryan Cutler> Prevent EchoServer from closing the client socket after writing batches

Authored-by: Bryan Cutler <cutlerb@gmail.com>
Signed-off-by: Bryan Cutler <cutlerb@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants