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

uploadAndFinish() hangs with no Internet when using OkHttp3Requestor #78

Closed
ashughes opened this issue Oct 26, 2016 · 11 comments
Closed

Comments

@ashughes
Copy link

In the sample code below, when no Internet connection is present and using OkHttp3Requestor, the first upload throws an exception (as expected), but the second upload never returns from uploadAndFinish().

DbxRequestConfig requestConfig = DbxRequestConfig.newBuilder(clientId)
        .withHttpRequestor(OkHttp3Requestor.INSTANCE)
        .build();
DbxClientV2 client = new DbxClientV2(requestConfig, accessToken)

File file1 = new File(localFilePath1);
File file2 = new File(localFilePath2);

FileInputStream in;

// Upload file1
try {
    in = new FileInputStream(file1);
    client.files().uploadBuilder(dropboxPath)
        .withMode(WriteMode.OVERWRITE)
        .withClientModified(new Date(file1.lastModified()))
        .uploadAndFinish(in);
} catch (Exception e) {
}

// Upload file2
try {
    in = new FileInputStream(file2);
    client.files().uploadBuilder(dropboxPath)
        .withMode(WriteMode.OVERWRITE)
        .withClientModified(new Date(file2.lastModified()))
        .uploadAndFinish(in); // <- HANGS FOREVER
} catch (Exception e) {
}

It seems as if once OkHttp throws an exception when it can't connect to the host, any subsequent request just hangs indefinitely.

This behavior does not occur when using the StandardHttpRequestor (instead, both upload operations throw an exception as expected).

@greg-db
Copy link
Contributor

greg-db commented Oct 26, 2016

Thanks for the report! I can't seem to reproduce this though. (I'm getting exceptions thrown for both file uploads when attempting this offline.) What versions of the Dropbox and OkHttp libraries do you have installed? Also, can you add some logging, such as for the exceptions in your catch blocks, and share the output? Thanks in advance!

@ashughes
Copy link
Author

Sorry, I was trying to represent what I thought was causing the issue into a simple example, but I might have over simplified it. The bottom line is that in whatever situation I'm in, when I use the OkHttp3Requestor, I'm getting stuck in uploadAndFinish(), but when I use the StandardHttpRequestor, I don't. I've let it sit for longer than the 2 minute default read/write timeout and tried toggling the Internet connection as well, but it doesn't ever return. I'll see if I can figure out a better example to reproduce the issue.

I'm using Dropbox 2.1.2 and OkHttp 3.4.1.

As an aside, are there significant advantages to using the OkHttp3Requestor over the StandardHttpRequestor? Or is it more personal preference?

@greg-db
Copy link
Contributor

greg-db commented Oct 27, 2016

Thanks! I just tried again with those specific versions and I still can't reproduce it with this code, so please do let us know if you can track that down.

And yes, it's up to you. For example, you may want to use OkHttp if your app already uses it for other HTTP use anyway, etc.

@krieb
Copy link

krieb commented Nov 2, 2016

The OkHttp requestors have to do some concurrency magic to allow streaming uploads. Whenever you call uploadAndFinish(in), a request is issued asynchronously through OkHttp with an empty PipedInputStream as the body. Then we copy over bytes from your input stream into the corresponding PipedOutputStream and register an asynchronous callback that allows us to wait for the request to finish. This way we don't have to buffer all bytes in memory before issuing the request.

To figure out what's going on, you should take a thread dump when the request hangs so we know where the threads are blocked. You can use the JDK jstack command:

$ jstack -F pid

My guess is that it's blocking on writes to the PipedOutputStream. Those writes are blocking if we filled up the 5MiB internal buffer for the piped stream. So if the OkHttp thread reading from the pipe died and didn't properly close the stream (or an exception was thrown during stream close), the writes could be hanging.

@krieb
Copy link

krieb commented Nov 2, 2016

@greg-db When you attempt to reproduce, make sure you use a file that's larger than 5MiB.

@greg-db
Copy link
Contributor

greg-db commented Nov 2, 2016

Thanks for the tip Karl! I can now reproduce this using a single upload of a file larger than 5MiB. We'll look into it.

@greg-db
Copy link
Contributor

greg-db commented Sep 20, 2017

This should be fixed in v3.0.4.

@atkawa7
Copy link

atkawa7 commented Jun 3, 2022

@greg-db @wclausen @joshafeinberg @lchen8

A simple program using uploadAndFinish() hangs and doesn't stop immediately after uploading a document. This happens when using OkHttp3Requestor. This is because dropbox uses simple asynchronous calls to upload documents. This issue is still persisting in all dropbox clients. The issue is related to dropbox not closing okhttp client12. JW suggests to use execute() instead of enqueue().

To prevent hanging use the following to close executorService

okHttpClient.dispatcher().executorService().shutdown();
okHttpClient.connectionPool().evictAll();

Footnotes

  1. square/okhttp#1739

  2. Okhttp Asynchronous Request

@greg-db
Copy link
Contributor

greg-db commented Jun 3, 2022

@atkawa7 Thanks for the report! We'll look into it.

@eyousefi
Copy link
Contributor

eyousefi commented Jun 3, 2022

@atkawa7 thanks for the report.
The dropbox java sdk provides two template starters for networking stack, OkHttp (OkHttp3Requestor) and Java's standard library HttpsURLConnection (StandardHttpRequestor). You can also extend HttpRequestor and provide your own impl as well.

For the default settings on OkHttp3Requestor, you can use OkHttp3Requestor.defaultOkHttpClientBuilder() and set your own dispatcher and connectionpool, for which you can tie to the applications lifecycle and manually kill as needed.

@greg-db also repro'd a similar issue without any requests inflight (possibly this), so this is not limited to uploadAndFinish().

TL;DR; if you need an immediate shutdown, configure the requestor with your own dispatcher and connection pool.

Adding some relevant links:
https://square.github.io/okhttp/4.x/okhttp/okhttp3/-ok-http-client/#shutdown-isnt-necessary

@atkawa7
Copy link

atkawa7 commented Jun 3, 2022

@eyousefi Thanks for the suggestions. Whilst the default StandardHttpRequestor doesn't have these issues I can't use it. I am currently using http interceptors on a spring boot service to log requests. I need to use OkHttp as the StandardHttpRequestor uses HttpsURLConnection which cannot be intercepted. Another client we have in our codebase is apache http client which I am seeing is not supported (I might be wrong). I think your suggestions of shutting down is a likely solution which I need to look into more.

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

6 participants