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

OkHttp causes JVM 11 to hang with HTTP/2 #5832

Closed
fastily opened this issue Feb 29, 2020 · 4 comments
Closed

OkHttp causes JVM 11 to hang with HTTP/2 #5832

fastily opened this issue Feb 29, 2020 · 4 comments
Labels
bug Bug in existing code
Milestone

Comments

@fastily
Copy link

fastily commented Feb 29, 2020

The following code

import okhttp3.OkHttpClient;
import okhttp3.Request;

public class A
{
	public static void main(String[] args) throws Throwable
	{
		System.out.println(
				new OkHttpClient.Builder().build().newCall(
						new Request.Builder().url("https://en.wikipedia.org/w/api.php").build()).execute());
	}
}

doesn't exit for several minutes in java 11. I'm using the 4.4.0 release of OkHttp.

Run and compile with javac A.java && java A

My environment (macOS Catalina 10.15.3):

$ java -version
openjdk version "11.0.2" 2019-01-15
OpenJDK Runtime Environment 18.9 (build 11.0.2+9)
OpenJDK 64-Bit Server VM 18.9 (build 11.0.2+9, mixed mode)

$ echo $BASH_VERSION
5.0.16(1)-release

Likely related to #4029, which wasn't actually fixed apparently.

@fastily fastily added the bug Bug in existing code label Feb 29, 2020
@swankjesse
Copy link
Collaborator

@fastily
Copy link
Author

fastily commented Feb 29, 2020

I'm aware that shutdown() can be used to exit immediately. What about the case where I have

// MyLibrary.java
import okhttp3.OkHttpClient;
import okhttp3.Request;

public class MyLibrary
{
	private static final OkHttpClient client = new OkHttpClient.Builder().build();

	public static String getData() throws Throwable
	{
		return client.newCall(new Request.Builder().url("https://en.wikipedia.org/w/api.php").build()).execute().body().string();
	}
}

and

// A.java
public class A
{
	public static void main(String[] args) throws Throwable
	{
		System.out.println(MyLibrary.getData());
	}
}

If I'm the author of MyLibrary.java, and if A.java is a program written by another developer consuming MyLibrary.java, then requiring a separate, explicit call to shutdown() to shutdown OkHttp is effectively a breaking change.

It becomes especially problematic if that other developer has written hundreds of standalone programs with MyLibrary.java as a dependency. Basically it ensures they will never upgrade.

I'd also like to point out that using OkHttp with HTTP/1.1 doesn't require shutdown() whereas HTTP/2 does. In terms of consistency, this is fairly unintuitive.

@yschimke
Copy link
Collaborator

PR to use a daemon thread for clients to be consistent for HTTP/2 and HTTP/1.1
#5834

@yschimke yschimke added this to the 4.5 milestone Feb 29, 2020
@swankjesse
Copy link
Collaborator

Fixed in 4.5.1-RC1.

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

No branches or pull requests

3 participants