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

Call.cancel() doesn’t cancel slow DNS lookups #2278

Closed
danvinokour opened this issue Jan 24, 2016 · 9 comments
Closed

Call.cancel() doesn’t cancel slow DNS lookups #2278

danvinokour opened this issue Jan 24, 2016 · 9 comments

Comments

@danvinokour
Copy link

I simulated network problems (100% packet loss), called Call.cancel() after 5 seconds and it actually took about 42 seconds for the execute method to throw IOException.

Sometimes i get exception after connect timeout (set on 3 seconds) but if connection was successful it takes very long time for cancel to work (if it really works and this is not the read timeout kicking in which is set to 25 seconds)

776 is the thread that suppose to cancel the call.
ModernAsyncTask #5 is the thread from which call is executed.

logs:
E/Censored ( 4870): [Thread-776] sleeping for 5000 ms
E/Censored ( 4870): [ModernAsyncTask #5] before execute
E/Censored ( 4870): [Thread-776] after cancel - took 5005 ms
E/Censored ( 4870): [ModernAsyncTask #5] IOException 42680ms | error - {}java.net.UnknownHostException: Unable to resolve host "Censored": No address associated with hostname

@swankjesse
Copy link
Collaborator

We have no mechanism to cancel during DNS.

@danvinokour
Copy link
Author

You already have this task: #2191
which will be a great help in this field.

Do you think that once we have access to deadline we will be covered and will be able to count on timeout?

Can you insert a mechanism to cancel during DNS?

@swankjesse
Copy link
Collaborator

At the moment the APIs Java gives us for DNS are quite limited. You could implement your own Dns class, possibly one that uses two threads so that the waiting thread can always break prematurely.

@danvinokour
Copy link
Author

Im no expert in this field but currently what im going to do is call.execute() in a different thread which will return an answer to Response[0] and if execute takes more than needed i will just kill this thread.

Maybe you could make something similar but more safe socket wise etc...?

Also, do you think that when okio's deadline will be reachable, will it definitely make the call exit immediately after the deadline expires?

@swankjesse
Copy link
Collaborator

Unfortunately OkHttp doesn’t use okio for DNS. That call doesn’t do I/O; it’s just a magical system call. It’d be pretty awesome to do a standalone DNS API, but that’d be potentially tricky, especially since you’d want to honor the local operating system’s cache & configuration.

@swankjesse
Copy link
Collaborator

(Your approach to using 2 threads is right. But at the moment OkHttp doesn’t signal the thread that’s doing DNS to break out prematurely. We’ll need to address this.)

@swankjesse swankjesse changed the title Call.cancel() take very long time Call.cancel() doesn’t cancel slow DNS lookups Jan 24, 2016
@JakeWharton
Copy link
Collaborator

Can this be closed as a dupe of #95?

@JakeWharton
Copy link
Collaborator

Or rather, should we think of #95 as expanded to encapsulate explicit cancel vs. timeout-based cancelation since they likely have similar fixes?

@swankjesse
Copy link
Collaborator

Sure.

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

No branches or pull requests

3 participants