Skip to content
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

Gnirehtet leaking connections and memory on TCP misuse #57

Closed
artnalban opened this issue Nov 13, 2017 · 3 comments
Closed

Gnirehtet leaking connections and memory on TCP misuse #57

artnalban opened this issue Nov 13, 2017 · 3 comments
Assignees
Labels

Comments

@artnalban
Copy link

Not sure if this is even important to you, but I have noticed that gnirehtet leaking connection and memory when trying to do TCP connections when UDP is required. I had to write internet connectivity checker, so I was trying to ping Google public DNS. Later turned out that I was trying to establish TCP connections instead of UDP.
In order to repro this run this in a loop:

Socket sock = new Socket();
SocketAddress sockaddr = new InetSocketAddress("8.8.8.8", 53);

sock.connect(sockaddr , timeoutMs);
sock.close();

Overtime time this will cause connections and memory to leak. This repros on both java and rust relay server implementations.

@rom1v rom1v added the bug label Nov 13, 2017
@rom1v rom1v self-assigned this Nov 13, 2017
rom1v added a commit that referenced this issue Nov 13, 2017
When the TCP connection received a FIN, it stayed in CLOSE WAIT state
without actually moving to LASK-ACK (cf diagram in RFC793:
<https://tools.ietf.org/html/rfc793#section-3.2>).

Since TCP connection _is_ the TCP peer (from the Android client point of
view), there is no need to wait for the network socket to be actually
closed, so just flag "remote closed" immediately, and bypass the
TCP CLOSE WAIT state.

Fixes <#57>.
@rom1v
Copy link
Collaborator

rom1v commented Nov 13, 2017

Thank you for your report.

Not sure if this is even important to you

Of course, it is :)

I have noticed that gnirehtet leaking connection and memory when trying to do TCP connections when UDP is required

In fact, it affects all TCP sockets : a close() from the Android-side left the TCP connection state in a "waiting state" indefinitely, which is quite a huge bug. Google DNS also listens on TCP, that's why you get the problem with it.

I just fixed it on dev branch. Could you test, please?

@artnalban
Copy link
Author

Hello, I've quickly tested java relay server today and haven't seen any connection to stack up. I was unable to test rust implementation because I couldn't build it.

Thanks.

@rom1v
Copy link
Collaborator

rom1v commented Mar 2, 2018

It's in the v2.2 release: https://github.com/Genymobile/gnirehtet/releases/tag/v2.2

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants