koush
commented
10 years ago Which device in particular? Are you on wifi or mobile network?
Open markshiz opened 10 years ago
koush
commented
10 years ago Which device in particular? Are you on wifi or mobile network?
markshiz
commented
10 years ago This device was on Wi-Fi. Galaxy Nexus.
markshiz
commented
10 years ago Basically the case to reproduce is to let a device sit on Wi-Fi long enough for the radio to power down. Keep the existing app in memory. At some point later, perhaps the next day or so, pick up the device again and begin using it. My guess is AsyncSocketMiddleware is reusing a socket it shouldn't for some reason. I haven't exactly been able to pin it down yet.
koush
commented
10 years ago Alright. TCP is weird in that you can't actually see if a socket has died unless the other end sends a fin packet or you attempt to write to the socket and it fails. Socket.io has a ping built into the protocol to address this exact situation. I can work around this by forcing sockets that are too old to be force closed.
markshiz
commented
10 years ago Yes. Doing a little more research, it looks as if the code is not properly handling keep-alive sockets after they have been closed by the server. I'm not seeing a socket TimeoutException() ever removing a socket from consideration for being recycled. It's also not closing the socket out. Looks like you require a response for that currently:
private void reportConnectedCompleted(FutureAsyncHttpResponse cancel, Exception ex, AsyncHttpResponseImpl response, AsyncHttpRequest request, final HttpConnectCallback callback) {
assert callback != null;
boolean complete;
if (ex != null) {
request.loge("Connection error", ex);
complete = cancel.setComplete(ex);
}
else {
request.logd("Connection successful");
complete = cancel.setComplete(response);
}
if (complete) {
callback.onConnectCompleted(ex, response);
assert ex != null || response.getSocket() == null || response.getDataCallback() != null || response.isPaused();
return;
}
if (response != null) {
// the request was cancelled, so close up shop, and eat any pending data
response.setDataCallback(new NullDataCallback());
response.close();
}
}There are options -
koush
commented
10 years ago The connect timeout itself will close the socket (whether or not if a response was received)
// set connect timeout
cancel.timeoutRunnable = new Runnable() {
@Override
public void run() {
// we've timed out, kill the connections
if (data.socketCancellable != null) {
data.socketCancellable.cancel();
if (data.socket != null)
data.socket.close();
}
reportConnectedCompleted(cancel, new TimeoutException(), null, request, callback);
}
};Since it is nio, it is always "reading". A read event will come in and with a zero length data indicating failure. I've heard that another viable option is to do a 0 byte write. I can maybe try that.
Though, as I mentioned, the easiest fix is to just not hang onto keep-alive sockets indefinitely and time them out after some period. Holding onto them forever is not the best idea anyways. Furthermore, while I'm in there, I can respect server responses with a keep-alive header (which specifies connection reuse parameters).
markshiz
commented
10 years ago I agree that part of the solution would be to not hold onto the sockets forever. You certainly don't want to do that.
The tricky part will be determining the timeout period. The header will inform you as to the host's constraints, which may be different from the Android device's. If idle long enough, the Android OS may kill the socket, too. Determining what that period is may be possible or impossible from device to device. I really don't know. That is why I suggested the other socket options or initiating a socket read/write to see if the socket works before using it to mitigate that risk.
koush
commented
10 years ago Yeah, I agree that working a zero byte write in there is a good plan.
koush
commented
10 years ago Part 1 of the fix... time them out
3168f78
swatigoel
commented
10 years ago @koush can you please update on this issue also?
nschwermann
commented
9 years ago I think this may be related to my issue https://github.com/koush/ion/issues/374
kaizenx
commented
7 years ago This bug just re-surfaced in 2.1.8 whereby Reusing keep-alive socket is stuck forever.
After an Ice Cream Sandwich device goes to sleep and becomes active again, it appears as if the cached keep-alive sockets are no longer valid. This results in the app using them basically being non-functional. I'm not sure what the fix for this is, but there may be a way to detect the state of the socket before reuse. See verbose logs below:
Not sure if it's related or not, but if I scroll up far enough in the logs, I can see a connection being reset.