close socket after a complete write in TcpClient output

[ganxiyun]

I have many java processes running in output=tcpclient mode, the process keeps running a bit long.

When I check the log in the server side (on which tcpclient connects to), I see several EOFException.

After some investigation, the socket might be closed in client side while the RemoteControlWriter is still writing data to server side, therefore the server receives partial data and throws EOFException.

I can see such EOFException happening almost every time in my case (Many connections with long running period), although not big problem, I create this PR trying to fix it. Please kindly have a review.

[ganxiyun]

Seems now it has build error which is irrelevant to the PR?

[Godin]

Seems now it has build error which is irrelevant to the PR?

yes it was unrelated - see #1249

[ganxiyun]

Anyone who can kindly review this?

[marchof]

Did you already test this fix? Does it solve the problem you observe?

[ganxiyun]

Hi @marchof I have manually tested local (not by unit test, I deliberately added some code to simulate multi-threading execution) and didn't observe the problem currently.

[marchof]

@ganxiyun I mean does this patch resolve the problem in your setup that you describe at the beginning?

[ganxiyun]

@marchof YES. After applying this patch, I didn't observe the problem currently.

[marchof]

@ganxiyun Ok, thanks for the info!

I'm a bit reluctant to merge this without fully understanding the impact. My questions are:

• TcpConnectionis also use in tcpserver mode. Do we have any sieffects here (deadlock)?
• The close() method is called during VM shutdown. Is it a good idea to potentially block the shutdown sequence?

[ganxiyun]

@marchof I could understand you concern.

TcpConnection is also use in tcpserver mode. Do we have any sieffects here (deadlock)?

I checked the code and class diagram (generated by Intellij) in TcpConnection, RemoteControlWriter, RuntimeData, ExecutionDataStore, and ExecutionData, there is no circular dependency, which breaks the Circular wait condition in deadlock. I think no deadlock here. For other sieffects, as you mentioned below, I will answer with my understanding.

The close() method is called during VM shutdown. Is it a good idea to potentially block the shutdown sequence?

If all data has been written to the socket before calling close(), there is no difference from the current implementation.
If it's in writing data sync block, I think waiting for writing is better, like flush the cache before closing. Usually it won't take long time. In case of write failure, exception will occur so the developers have the chance to know the problem.

multi-threading is not easy to test etc., your review opinion values a lot.