-
Notifications
You must be signed in to change notification settings - Fork 9.2k
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
Idle connection breaks when turning data off and on. #4079
Comments
Clarification: Here are the reproduction steps. I didn't include them in the original post because they are already in the repo I linked, but it's probably useful to have them visible here as well): Reproduction
|
@roberthoenig Thanks for the thorough reproduction, I'll take a look in next couple of days if no-one beats me to it. If you get any more insight please post here as there are a number of issues seen that are some form of Android network disconnects not observed after airplane mode. |
Just a quick note: I don't actually know that disconnecting corrupts the idle connections. All I observed is that it they get corrupted somehwere between turning data off and on. |
…ebook#19709). This bug is probably actually a bug in OkHttp: square/okhttp#4079 Both issues linked above contain extensive details about the issue, its likely origins and how to reproduce it. A short summary of the issue and the fix in this commit: On Android, disconnecting from the network somehow corrupts the idle connections in okhttp clients. New requests made over these clients fail. This commit works around that bug by clearing the idle connection pool when Android disconnects from the network.
…book#19709. This bug is probably actually a bug in OkHttp: square/okhttp#4079 Both issues linked above contain extensive details about the issue, its likely origins and how to reproduce it. A short summary of the issue and the fix in this commit: On Android, disconnecting from the network somehow corrupts the idle connections in okhttp clients. New requests made over these clients fail. This commit works around that bug by clearing the idle connection pool of each client when Android disconnects from the network.
@roberthoenig Thanks for the app, unfortunately I couldn't really reproduce what you are seeing. Is it some particular android version you are testing with? Is it only on devices or emulators? I do see a delay in the request after reconnect, but it succeeds and then following requests work fine. I'm sure I'm just doing something differently to you, and I've seen similar problems myself. I've got a similar test app https://github.com/yschimke/okhttp-testapp |
I'm testing it on an emulator with the following details:
It should be a standard configuration of a Nexus S with API level 23. Nothing spcial about the choice of device. The API level is the target API of React Native. My
The original reports of this issue zulip/zulip-mobile#2287 zulip/zulip-mobile#2310 zulip/zulip-mobile#2315 were on real devices. I only reproduced the issue on an emulator. Didn't try myself on a real device yet.
Hmm, how long is the delay? One thing noteworthy about my repro steps is that I turn the network off and on with
Also, I just tried to reproduce it myself again with the steps outlined above:
Step 1 wasn't actually necessary for things to break. Might be for others, though. |
…book#19709. This bug is probably actually a bug in OkHttp: square/okhttp#4079 Both issues linked above contain extensive details about the issue, its likely origins and how to reproduce it. A short summary of the issue and the fix in this commit: On Android, disconnecting from the network somehow corrupts the idle connections in okhttp clients. New requests made over these clients fail. This commit works around that bug by clearing the idle connection pool of each client when Android disconnects from the network.
…book#19709. This bug is probably actually a bug in OkHttp: square/okhttp#4079 Both issues linked above contain extensive details about the issue, its likely origins and how to reproduce it. A short summary of the issue and the fix in this commit: On Android, disconnecting from the network somehow corrupts the idle connections in okhttp clients. New requests made over these clients fail. This commit works around that bug by clearing the idle connection pool of each client when Android disconnects from the network.
…book#19709. This bug is probably actually a bug in OkHttp: square/okhttp#4079 Both issues linked above contain extensive details about the issue, its likely origins and how to reproduce it. A short summary of the issue and the fix in this commit: On Android, disconnecting from the network somehow corrupts the idle connections and ongoing calls in okhttp clients. New requests made over these clients fail. This commit works around that bug by evicting the idle connection pool and cancelling all ongoing calls of each client when we receive a DISCONNECTED or CONNECTING event (we don't know yet if only one or both of them cause the issue). Cancelling all calls is aggressive, but when a device disconnects any ongoing calls can fail anyway, so an app has to expect this scenario.
…book#19709. This bug is probably actually a bug in OkHttp: square/okhttp#4079 Both issues linked above contain extensive details about the issue, its likely origins and how to reproduce it. A short summary of the issue and the fix in this commit: On Android, disconnecting from the network somehow corrupts the idle connections and ongoing calls in okhttp clients. New requests made over these clients fail. This commit works around that bug by evicting the idle connection pool and cancelling all ongoing calls of each client when we receive a DISCONNECTED or CONNECTING event (we don't know yet if only one or both of them cause the issue). Cancelling all calls is aggressive, but when a device disconnects any ongoing calls can fail anyway, so an app has to expect this scenario.
…book#19709. This bug is probably actually a bug in OkHttp: square/okhttp#4079 Both issues linked above contain extensive details about the issue, its likely origins and how to reproduce it. A short summary of the issue and the fix in this commit: On Android, disconnecting from the network somehow corrupts the idle connections and ongoing calls in okhttp clients. New requests made over these clients fail. This commit works around that bug by evicting the idle connection pool when we receive a DISCONNECTED or CONNECTING event (we don't know yet if only one or both of them cause the issue). Technically, to fully fix this issue, we would also need to cancel all ongoing calls. However, cancelling all ongoing calls is aggressive, and not always desired (when the app disconnects only for a short time, ongoing calls might still succeed). In practice, just evicting idle connections results in this issue occurring less often, so let's go with that for now.
Does it time out? Seems like a bug in Android. |
Unclear what action to take here. |
I've been getting this as well. There are other bug reports here that discuss this |
I have the same issue, when to fix it? |
Presumably the nicest fix would be a thing that observes Android’s network stack and applies changes to our connection pool. That could be a code sample or a gist! |
@swankjesse forking off #4366, because I had some thoughts I was experimenting with previously... |
Probably worth retesting after #5920 which is in 4.5, I'm hopeful that fixes a ton of connectivity issues with dodgy Android networking. Anyone seeing with 4.5? |
Just for information: |
Is this a duplicate of #3278 ? |
@lyind It may well be! (Speaking as a colleague of this issue's reporter.) Thanks for spotting that connection. |
Dupe of #3278 |
I first thought this to be an issue with React Native and filed an issue there: facebook/react-native#19709
However, further debugging makes me believe that the issue might lie within okhttp.
I created a tiny Github repo that reproduces this bug with okhttp in an Android app:
https://github.com/roberthoenig/react-native-fetch-bug
(Don't get confused by the repo name react-native-fetch-bug: The example doesn't use React Native at all, it's just where I encountered this bug first.)
The repo's README.md also contains steps to reproduce the issue.
Here is my theory of what's going on:
The text was updated successfully, but these errors were encountered: