Open kayrus opened 2 years ago
I'd recommend regularly sending Noop
commands and pruning connections where that fails. Or send Noop
when extracting a connection from the pool.
@emersion sending Noop()
adds an additional latency if you call it right before returning a valid connection from a pool. Regular Noop()
sending also adds code complexity, useless loops and CPU cycles. Besides you closed the Noop()
PR.
At the end It'd be more cleaner to know that connection is closed in event based manner. And yes, I'd still would like to have Noop() method in Session interface independently, because smtp.Session
interface is used in my app along with go-smtp-proxy
repo.
I'm implementing SMTP connections pool in my app (see #154) and I already faced two issues when a remote SMTP server closes an idle connection due to timeout:
CLOSE_WAIT
consuming the OS descriptor unless you run a command within the client's public methods or explicitly close the connection which doesn't make sense for a connections pool.SMTP client is not aware about the closed connection, because textproto uses bufio.Reader under the hood and due to client's Command/Response approach bufio buffers the timeout response until the next Command/Response action.
See a telnet example:
See the last
451 4.4.2 Timeout waiting for data from client.
response. It is sent by remote server and not recognized by the SMTP client until you call any other command.Not sure if this is a
go-smtp
problem, but an upstream textproto package issue. Nevertheless this unexpected behavior has a workaround innet/http.Transport
package: https://github.com/golang/go/blob/e7c56fe9948449a3710b36c22c02d57c215d1c10/src/net/http/transport.go#L2092I developed a couple of workarounds. One is based on substituting the
c.Text.Reader.R
reader and writeAhead approach, another is based on readAhead approach by implementing a customRead
method and wrapping thec.conn
. Both workarounds work more or less, but the second one is more cleaner in terms of the code. The workaround detectsio.EOF
right after the remote server closes the connection and it is possible to close thec.Text
to fully close the descriptor.Unfortunately when a plain connection is upgraded to StartTLS, some problems popup in rare cases. Solving these problems require overengineering with channels, selectors, etc. Not sure if I'm on the right way.
Is it the first time you hear about this problem? Does it make sense to solve it?