New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Dealing with half-opened TCP connections #622

Closed
korobochka opened this Issue Apr 16, 2014 · 8 comments

Comments

Projects
None yet
2 participants
@korobochka

korobochka commented Apr 16, 2014

Hello,
I am using connectTCP/listenTCP in my application and currently came across "half open connection problem" (info here: http://blog.stephencleary.com/2009/05/detection-of-half-open-dropped.html): my reading Task never stops if no information is sent(with error) to remote host.
Basically, I want to detect network problems like router reboots or disconnects, and remote system crashes. This can be implemented on application level, but article above suggests that good solution is TCP Keep-Alive.

So, questions here:

  1. How do you suggest to deal with half-opened connections?
  2. Does vibe.d support TCP Keep-Alive enabling and configuring?

Thank you.

@s-ludwig

This comment has been minimized.

Show comment
Hide comment
@s-ludwig

s-ludwig Apr 17, 2014

Member

It always depends, but usually I'd deal with this issue implicitly by implementing a read timeout, either explicitly using TCPConnection.waitForData, or implicitly with TCPConnection.readTimeout. A read timeout is usually enough, because the writing side, assuming a properly implemented request/response protocol, will notice the broken connection anyway. On top of that, as you said, many application level protocols support a ping-like message that can be sent manually - this is the generally recommended practice.

Currently there is no (clean) way to enable TCP keep-alive, but I'll add that for the next version. Beware, though, that TCP keep-alives may be unreliable if sent over the internet, so at worst they may have a similar effect as a read timeout.

Member

s-ludwig commented Apr 17, 2014

It always depends, but usually I'd deal with this issue implicitly by implementing a read timeout, either explicitly using TCPConnection.waitForData, or implicitly with TCPConnection.readTimeout. A read timeout is usually enough, because the writing side, assuming a properly implemented request/response protocol, will notice the broken connection anyway. On top of that, as you said, many application level protocols support a ping-like message that can be sent manually - this is the generally recommended practice.

Currently there is no (clean) way to enable TCP keep-alive, but I'll add that for the next version. Beware, though, that TCP keep-alives may be unreliable if sent over the internet, so at worst they may have a similar effect as a read timeout.

@korobochka

This comment has been minimized.

Show comment
Hide comment
@korobochka

korobochka Apr 17, 2014

But what if I want to wrap connection into some Stream, for example SSLStream? There are no read timeouts there.

Yes, I know about this issue. Still, TCP keep-alive for next version would be great.

korobochka commented Apr 17, 2014

But what if I want to wrap connection into some Stream, for example SSLStream? There are no read timeouts there.

Yes, I know about this issue. Still, TCP keep-alive for next version would be great.

@s-ludwig

This comment has been minimized.

Show comment
Hide comment
@s-ludwig

s-ludwig Apr 17, 2014

Member

Using the TCP read timeout will still work for wrapped streams as normal, and for the waitForData approach, calling waitForData on the TCP connection when wrapped_stream.dataAvailableForRead returns false is almost always a correct approach, too (this is what the HTTP server does for keep-alive connections).

Member

s-ludwig commented Apr 17, 2014

Using the TCP read timeout will still work for wrapped streams as normal, and for the waitForData approach, calling waitForData on the TCP connection when wrapped_stream.dataAvailableForRead returns false is almost always a correct approach, too (this is what the HTTP server does for keep-alive connections).

@korobochka

This comment has been minimized.

Show comment
Hide comment
@korobochka

korobochka Apr 17, 2014

And the last questions: will blocked wrapped_stream.read throw if I close underlying socket from some other task after unsuccessful write(got exception) to wrapped_stream?

Does exception on read/write to Stream indicate, that connection is broken? Or should I retry several times before closing it?

What is the correct way of ending session?

TCPConnection c = ...;
SSLStream s = ...; // wrap c
// ... work with stream s
s.finalize();
c.finalize(); // do I need to call this?
c.close();

korobochka commented Apr 17, 2014

And the last questions: will blocked wrapped_stream.read throw if I close underlying socket from some other task after unsuccessful write(got exception) to wrapped_stream?

Does exception on read/write to Stream indicate, that connection is broken? Or should I retry several times before closing it?

What is the correct way of ending session?

TCPConnection c = ...;
SSLStream s = ...; // wrap c
// ... work with stream s
s.finalize();
c.finalize(); // do I need to call this?
c.close();
@s-ludwig

This comment has been minimized.

Show comment
Hide comment
@s-ludwig

s-ludwig Apr 17, 2014

Member

And the last questions: will blocked wrapped_stream.read throw if I close underlying socket from some other task after unsuccessful write(got exception) to wrapped_stream?

If the connection got terminated due to an unsuccessful write, that should be the case, but I'll have to double check test this to see how libevent behaves in detail. However, actively closing the connection from one task while reading or writing in another is currently not allowed and triggers an assertion (although I don't see a reason why that couldn't be made working, I'll look into that, too).

What is the correct way of ending session?

When close is called right afterwards, an explicit call to finalize isn't necessary (but doesn't do harm either), but generally finalize must indeed be called on each wrapped stream in reverse order. So the example is correct.

Member

s-ludwig commented Apr 17, 2014

And the last questions: will blocked wrapped_stream.read throw if I close underlying socket from some other task after unsuccessful write(got exception) to wrapped_stream?

If the connection got terminated due to an unsuccessful write, that should be the case, but I'll have to double check test this to see how libevent behaves in detail. However, actively closing the connection from one task while reading or writing in another is currently not allowed and triggers an assertion (although I don't see a reason why that couldn't be made working, I'll look into that, too).

What is the correct way of ending session?

When close is called right afterwards, an explicit call to finalize isn't necessary (but doesn't do harm either), but generally finalize must indeed be called on each wrapped stream in reverse order. So the example is correct.

@korobochka

This comment has been minimized.

Show comment
Hide comment
@korobochka

korobochka Apr 17, 2014

Thank you very much for help!

I will leave this issue open as a feature request for:

  • Implementing keep-alive enabling and configuring for TCP connections
  • Closing some Stream should be possible while other tasks are blocked on reading from it. That other tasks should get an exception.

korobochka commented Apr 17, 2014

Thank you very much for help!

I will leave this issue open as a feature request for:

  • Implementing keep-alive enabling and configuring for TCP connections
  • Closing some Stream should be possible while other tasks are blocked on reading from it. That other tasks should get an exception.
@korobochka

This comment has been minimized.

Show comment
Hide comment
@korobochka

korobochka Apr 18, 2014

Using the TCP read timeout will still work for wrapped streams as normal, and for the waitForData approach, calling waitForData on the TCP connection when wrapped_stream.dataAvailableForRead returns false is almost always a correct approach, too (this is what the HTTP server does for keep-alive connections).

But what if we call waitForData, then wrapped_stream.read(buffer[0..64]), it reads (for example) 32 bytes without blocking, blocks and then connection dies? I think in this case server will still end up in half-open state... Am I correct? And is killing reading Task explicitly required in this situation?

korobochka commented Apr 18, 2014

Using the TCP read timeout will still work for wrapped streams as normal, and for the waitForData approach, calling waitForData on the TCP connection when wrapped_stream.dataAvailableForRead returns false is almost always a correct approach, too (this is what the HTTP server does for keep-alive connections).

But what if we call waitForData, then wrapped_stream.read(buffer[0..64]), it reads (for example) 32 bytes without blocking, blocks and then connection dies? I think in this case server will still end up in half-open state... Am I correct? And is killing reading Task explicitly required in this situation?

@s-ludwig

This comment has been minimized.

Show comment
Hide comment
@s-ludwig

s-ludwig Apr 19, 2014

Member

Yes killing/interrupting the task would be necessary there, but setting a TCPConnection.readTimeout would make this much simpler. read will then simply throw an exception once no data arrives anymore.

Member

s-ludwig commented Apr 19, 2014

Yes killing/interrupting the task would be necessary there, but setting a TCPConnection.readTimeout would make this much simpler. read will then simply throw an exception once no data arrives anymore.

@s-ludwig s-ludwig added this to the 0.7.20 milestone May 16, 2014

@s-ludwig s-ludwig closed this in 7d1f35e May 17, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment