-
Notifications
You must be signed in to change notification settings - Fork 19
Fix a file descriptor leak with wrong user:pass #133
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
Conversation
ae48631
to
10febcb
Compare
It seems that we cannot do so much attemps to connect on travis-ci. We should either set SO_REUSEADDR or disable these test cases on travis-ci. |
SO_REUSEADDR does not help. I'll fix and then add reviewers. |
d14693b
to
e466400
Compare
e466400
to
3f3a004
Compare
For the record: It is possible to fix CI with SO_REUSEADDR, but we need either lean on a test order or set it everywhere. I decided to set SO_LINGER to 0 for outgoing connections in tests where outgoing connections are created intensively. |
3f3a004
to
0246cca
Compare
NB: PR #135 fixes the problem too. |
I'm going to remove |
Set SO_LINGER to 0 in tests that produce many outgoing connections, because so many sockets in the TIME_WAIT state can lead to 'java.net.NoRouteToHostException: Cannot assign requested address (Address not available)' exceptions on Travis-CI. Fixes #132.
0246cca
to
e407078
Compare
@ponomarevDmitri complains about several things (thanks for pointers!):
Hm. A lifetime of these streams ( Things become a way different if we'll need to handle several sockets (and so several streams) within one tarantool client. It may be needed in the scope of #34 (see 'way 3' here). So preliminary refactoring (at least factor out functions for writing and reading packages from using saved streams) seems to be right way to proceed if we'll choose an implementation variant of #34, which use several sockets simultaneously inside tarantool client. Maybe it also improves readability of the code even if we'll not going to implement #34 in this way. I cannot say something about that right now. So I prefer to fix the bug w/o such refactoring here. About the test case that lean on an OS file descriptor limit. I investigated several other ways to verify we don't lose some fd. In short:
So I prefer to stick with the current approach: create many connections and verify we don't reach a fds limit. |
Set SO_LINGER to 0 in tests that produce many outgoing connections,
because so many sockets in the TIME_WAIT state can lead to
'java.net.NoRouteToHostException: Cannot assign requested address
(Address not available)' exceptions on Travis-CI.
Fixes #132.