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

Close HTTP client owned by MinioClient #1546

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 8 additions & 3 deletions api/src/main/java/io/minio/MinioAsyncClient.java
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,8 @@ private MinioAsyncClient(
boolean useVirtualStyle,
String region,
Provider provider,
OkHttpClient httpClient) {
OkHttpClient httpClient,
boolean closeHttpClient) {
super(
baseUrl,
awsS3Prefix,
Expand All @@ -148,7 +149,8 @@ private MinioAsyncClient(
useVirtualStyle,
region,
provider,
httpClient);
httpClient,
closeHttpClient);
}

protected MinioAsyncClient(MinioAsyncClient client) {
Expand Down Expand Up @@ -3338,10 +3340,12 @@ public MinioAsyncClient build() {
"Region missing in Amazon S3 China endpoint " + this.baseUrl);
}

boolean closeHttpClient = false;
if (this.httpClient == null) {
this.httpClient =
HttpUtils.newDefaultHttpClient(
DEFAULT_CONNECTION_TIMEOUT, DEFAULT_CONNECTION_TIMEOUT, DEFAULT_CONNECTION_TIMEOUT);
closeHttpClient = true;
}

return new MinioAsyncClient(
Expand All @@ -3352,7 +3356,8 @@ public MinioAsyncClient build() {
useVirtualStyle,
region,
provider,
httpClient);
httpClient,
closeHttpClient);
}
}
}
9 changes: 8 additions & 1 deletion api/src/main/java/io/minio/MinioClient.java
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@
* .build();
* }</pre>
*/
public class MinioClient {
public class MinioClient implements AutoCloseable {
private MinioAsyncClient asyncClient = null;

private MinioClient(MinioAsyncClient asyncClient) {
Expand Down Expand Up @@ -2450,6 +2450,13 @@ public void setAwsS3Prefix(String awsS3Prefix) {
asyncClient.setAwsS3Prefix(awsS3Prefix);
}

@Override
public void close() throws Exception {
if (asyncClient != null) {
asyncClient.close();
}
}

public static Builder builder() {
return new Builder();
}
Expand Down
38 changes: 37 additions & 1 deletion api/src/main/java/io/minio/S3Base.java
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@
import okhttp3.ResponseBody;

/** Core S3 API client. */
public abstract class S3Base {
public abstract class S3Base implements AutoCloseable {
static {
try {
RequestBody.create(new byte[] {}, null);
Expand Down Expand Up @@ -136,7 +136,10 @@ public abstract class S3Base {
protected String region;
protected Provider provider;
protected OkHttpClient httpClient;
protected boolean closeHttpClient;

/** @deprecated This method is no longer supported. */
@Deprecated
protected S3Base(
HttpUrl baseUrl,
String awsS3Prefix,
Expand All @@ -146,6 +149,28 @@ protected S3Base(
String region,
Provider provider,
OkHttpClient httpClient) {
this(
baseUrl,
awsS3Prefix,
awsDomainSuffix,
awsDualstack,
useVirtualStyle,
region,
provider,
httpClient,
false);
}

protected S3Base(
HttpUrl baseUrl,
String awsS3Prefix,
String awsDomainSuffix,
boolean awsDualstack,
boolean useVirtualStyle,
String region,
Provider provider,
OkHttpClient httpClient,
boolean closeHttpClient) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This breaks backward compatibility.

You would need to create another constructor and have this closeHttpClient argument and call the newly added constructor in this constructor.

Finally deprecate this constructor.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good catch, fixed!

this.baseUrl = baseUrl;
this.awsS3Prefix = awsS3Prefix;
this.awsDomainSuffix = awsDomainSuffix;
Expand All @@ -154,6 +179,7 @@ protected S3Base(
this.region = region;
this.provider = provider;
this.httpClient = httpClient;
this.closeHttpClient = closeHttpClient;
}

/** @deprecated This method is no longer supported. */
Expand Down Expand Up @@ -182,6 +208,7 @@ protected S3Base(
this.region = region;
this.provider = provider;
this.httpClient = httpClient;
this.closeHttpClient = false;
}

protected S3Base(S3Base client) {
Expand All @@ -193,6 +220,7 @@ protected S3Base(S3Base client) {
this.region = client.region;
this.provider = client.provider;
this.httpClient = client.httpClient;
this.closeHttpClient = client.closeHttpClient;
}

/** Check whether argument is valid or not. */
Expand Down Expand Up @@ -3757,4 +3785,12 @@ protected CompletableFuture<UploadPartCopyResponse> uploadPartCopyAsync(
}
});
}

@Override
public void close() throws Exception {
if (closeHttpClient) {
httpClient.dispatcher().executorService().shutdown();
httpClient.connectionPool().evictAll();
}
}
}
2 changes: 1 addition & 1 deletion functional/TestUserAgent.java
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@

public class TestUserAgent {
public static void main(String[] args) throws Exception {
MinioClient client = MinioClient.builder().endpoint("http://example.org").build();
MinioClient client = MinioClient.builder().endpoint("http://httpbin.org").build();
ByteArrayOutputStream baos = new ByteArrayOutputStream();
client.traceOn(baos);
client.bucketExists(BucketExistsArgs.builder().bucket("any-bucket-name-works").build());
Expand Down
Loading