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

[MRESOLVER-464] Workaround for JDK-822647 bug #408

Merged
merged 3 commits into from
Jan 17, 2024
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
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@
import java.util.HashMap;
import java.util.Locale;
import java.util.Map;
import java.util.concurrent.Semaphore;
import java.util.function.Supplier;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
Expand All @@ -64,8 +65,7 @@
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import static org.eclipse.aether.transport.jdk.JdkTransporterConfigurationKeys.CONFIG_PROP_HTTP_VERSION;
import static org.eclipse.aether.transport.jdk.JdkTransporterConfigurationKeys.DEFAULT_HTTP_VERSION;
import static org.eclipse.aether.transport.jdk.JdkTransporterConfigurationKeys.*;

/**
* JDK Transport using {@link HttpClient}.
Expand Down Expand Up @@ -117,6 +117,8 @@ final class JdkTransporter extends AbstractTransporter implements HttpTransporte

private final Boolean expectContinue;

private final Semaphore maxConcurrentRequests;

JdkTransporter(RepositorySystemSession session, RemoteRepository repository, int javaVersion)
throws NoTransporterException {
try {
Expand Down Expand Up @@ -180,6 +182,12 @@ final class JdkTransporter extends AbstractTransporter implements HttpTransporte
}
}

this.maxConcurrentRequests = new Semaphore(ConfigUtils.getInteger(
session,
DEFAULT_MAX_CONCURRENT_REQUESTS,
CONFIG_PROP_MAX_CONCURRENT_REQUESTS + "." + repository.getId(),
CONFIG_PROP_MAX_CONCURRENT_REQUESTS));

this.headers = headers;
this.client = getOrCreateClient(session, repository, javaVersion);
}
Expand Down Expand Up @@ -211,7 +219,7 @@ protected void implPeek(PeekTask task) throws Exception {
.method("HEAD", HttpRequest.BodyPublishers.noBody());
headers.forEach(request::setHeader);
try {
HttpResponse<Void> response = client.send(request.build(), HttpResponse.BodyHandlers.discarding());
HttpResponse<Void> response = send(request.build(), HttpResponse.BodyHandlers.discarding());
if (response.statusCode() >= MULTIPLE_CHOICES) {
throw new HttpTransporterException(response.statusCode());
}
Expand Down Expand Up @@ -244,7 +252,7 @@ protected void implGet(GetTask task) throws Exception {
}

try {
response = client.send(request.build(), HttpResponse.BodyHandlers.ofInputStream());
response = send(request.build(), HttpResponse.BodyHandlers.ofInputStream());
if (response.statusCode() >= MULTIPLE_CHOICES) {
closeBody(response);
if (resume && response.statusCode() == PRECONDITION_FAILED) {
Expand Down Expand Up @@ -352,7 +360,7 @@ protected void implPut(PutTask task) throws Exception {
request.method("PUT", HttpRequest.BodyPublishers.ofFile(tempFile.getPath()));

try {
HttpResponse<Void> response = client.send(request.build(), HttpResponse.BodyHandlers.discarding());
HttpResponse<Void> response = send(request.build(), HttpResponse.BodyHandlers.discarding());
if (response.statusCode() >= MULTIPLE_CHOICES) {
throw new HttpTransporterException(response.statusCode());
}
Expand All @@ -362,6 +370,16 @@ protected void implPut(PutTask task) throws Exception {
}
}

private <T> HttpResponse<T> send(HttpRequest request, HttpResponse.BodyHandler<T> responseBodyHandler)
throws IOException, InterruptedException {
maxConcurrentRequests.acquire();
try {
return client.send(request, responseBodyHandler);
} finally {
maxConcurrentRequests.release();
}
}

@Override
protected void implClose() {
// no-op
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,4 +43,19 @@ private JdkTransporterConfigurationKeys() {}
public static final String CONFIG_PROP_HTTP_VERSION = CONFIG_PROPS_PREFIX + "httpVersion";

public static final String DEFAULT_HTTP_VERSION = "HTTP_2";

/**
* The hard limit of maximum concurrent requests JDK transport can do. This is a workaround for the fact, that in
* HTTP/2 mode, JDK HttpClient initializes this value to Integer.MAX_VALUE (!) and lowers it on first response
* from the remote server (but it may be too late). See JDK bug
* <a href="https://bugs.openjdk.org/browse/JDK-8225647">JDK-8225647</a> for details.
*
* @configurationSource {@link RepositorySystemSession#getConfigProperties()}
* @configurationType {@link java.lang.Integer}
* @configurationDefaultValue {@link #DEFAULT_MAX_CONCURRENT_REQUESTS}
* @configurationRepoIdSuffix Yes
*/
public static final String CONFIG_PROP_MAX_CONCURRENT_REQUESTS = CONFIG_PROPS_PREFIX + "maxConcurrentRequests";

public static final int DEFAULT_MAX_CONCURRENT_REQUESTS = 100;
}
23 changes: 12 additions & 11 deletions src/site/markdown/configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -112,17 +112,18 @@ under the License.
| 85. | `"aether.transport.http.userAgent"` | `java.lang.String` | The user agent that repository connectors should report to servers. | `"Aether"` | | No | Session Configuration |
| 86. | `"aether.transport.https.securityMode"` | `java.lang.String` | The mode that sets HTTPS transport "security mode": to ignore any SSL errors (certificate validity checks, hostname verification). The default value is . | `"default"` | 1.9.6 | Yes | Session Configuration |
| 87. | `"aether.transport.jdk.httpVersion"` | `java.lang.String` | Use string representation of HttpClient version enum "HTTP_2" or "HTTP_1_1" to set default HTTP protocol to use. | `"HTTP_2"` | 2.0.0 | Yes | Session Configuration |
| 88. | `"aether.transport.wagon.config"` | `java.lang.Object` | The configuration to use for the Wagon provider. | - | | Yes | Session Configuration |
| 89. | `"aether.transport.wagon.perms.dirMode"` | `java.lang.String` | Octal numerical notation of permissions to set for newly created directories. Only considered by certain Wagon providers. | - | | Yes | Session Configuration |
| 90. | `"aether.transport.wagon.perms.fileMode"` | `java.lang.String` | Octal numerical notation of permissions to set for newly created files. Only considered by certain Wagon providers. | - | | Yes | Session Configuration |
| 91. | `"aether.transport.wagon.perms.group"` | `java.lang.String` | Group which should own newly created directories/files. Only considered by certain Wagon providers. | - | | Yes | Session Configuration |
| 92. | `"aether.trustedChecksumsSource.sparseDirectory"` | `java.lang.Boolean` | Is checksum source enabled? | `false` | 1.9.0 | No | Session Configuration |
| 93. | `"aether.trustedChecksumsSource.sparseDirectory.basedir"` | `java.lang.String` | The basedir where checksums are. If relative, is resolved from local repository root. | `".checksums"` | 1.9.0 | No | Session Configuration |
| 94. | `"aether.trustedChecksumsSource.sparseDirectory.originAware"` | `java.lang.Boolean` | Is source origin aware? | `true` | 1.9.0 | No | Session Configuration |
| 95. | `"aether.trustedChecksumsSource.summaryFile"` | `java.lang.Boolean` | Is checksum source enabled? | `false` | 1.9.0 | No | Session Configuration |
| 96. | `"aether.trustedChecksumsSource.summaryFile.basedir"` | `java.lang.String` | The basedir where checksums are. If relative, is resolved from local repository root. | `".checksums"` | 1.9.0 | No | Session Configuration |
| 97. | `"aether.trustedChecksumsSource.summaryFile.originAware"` | `java.lang.Boolean` | Is source origin aware? | `true` | 1.9.0 | No | Session Configuration |
| 98. | `"aether.updateCheckManager.sessionState"` | `java.lang.String` | Manages the session state, i.e. influences if the same download requests to artifacts/metadata will happen multiple times within the same RepositorySystemSession. If "enabled" will enable the session state. If "bypass" will enable bypassing (i.e. store all artifact ids/metadata ids which have been updates but not evaluating those). All other values lead to disabling the session state completely. | `"enabled"` | | No | Session Configuration |
| 88. | `"aether.transport.jdk.maxConcurrentRequests"` | `java.lang.Integer` | The hard limit of maximum concurrent requests JDK transport can do. This is a workaround for the fact, that in HTTP/2 mode, JDK HttpClient initializes this value to Integer.MAX_VALUE (!) and lowers it on first response from the remote server (but it may be too late). See JDK bug <a href="https://bugs.openjdk.org/browse/JDK-8225647">JDK-8225647</a> for details. | `100` | 2.0.0 | Yes | Session Configuration |
| 89. | `"aether.transport.wagon.config"` | `java.lang.Object` | The configuration to use for the Wagon provider. | - | | Yes | Session Configuration |
| 90. | `"aether.transport.wagon.perms.dirMode"` | `java.lang.String` | Octal numerical notation of permissions to set for newly created directories. Only considered by certain Wagon providers. | - | | Yes | Session Configuration |
| 91. | `"aether.transport.wagon.perms.fileMode"` | `java.lang.String` | Octal numerical notation of permissions to set for newly created files. Only considered by certain Wagon providers. | - | | Yes | Session Configuration |
| 92. | `"aether.transport.wagon.perms.group"` | `java.lang.String` | Group which should own newly created directories/files. Only considered by certain Wagon providers. | - | | Yes | Session Configuration |
| 93. | `"aether.trustedChecksumsSource.sparseDirectory"` | `java.lang.Boolean` | Is checksum source enabled? | `false` | 1.9.0 | No | Session Configuration |
| 94. | `"aether.trustedChecksumsSource.sparseDirectory.basedir"` | `java.lang.String` | The basedir where checksums are. If relative, is resolved from local repository root. | `".checksums"` | 1.9.0 | No | Session Configuration |
| 95. | `"aether.trustedChecksumsSource.sparseDirectory.originAware"` | `java.lang.Boolean` | Is source origin aware? | `true` | 1.9.0 | No | Session Configuration |
| 96. | `"aether.trustedChecksumsSource.summaryFile"` | `java.lang.Boolean` | Is checksum source enabled? | `false` | 1.9.0 | No | Session Configuration |
| 97. | `"aether.trustedChecksumsSource.summaryFile.basedir"` | `java.lang.String` | The basedir where checksums are. If relative, is resolved from local repository root. | `".checksums"` | 1.9.0 | No | Session Configuration |
| 98. | `"aether.trustedChecksumsSource.summaryFile.originAware"` | `java.lang.Boolean` | Is source origin aware? | `true` | 1.9.0 | No | Session Configuration |
| 99. | `"aether.updateCheckManager.sessionState"` | `java.lang.String` | Manages the session state, i.e. influences if the same download requests to artifacts/metadata will happen multiple times within the same RepositorySystemSession. If "enabled" will enable the session state. If "bypass" will enable bypassing (i.e. store all artifact ids/metadata ids which have been updates but not evaluating those). All other values lead to disabling the session state completely. | `"enabled"` | | No | Session Configuration |


All properties which have `yes` in the column `Supports Repo ID Suffix` can be optionally configured specifically for a repository id. In that case the configuration property needs to be suffixed with a period followed by the repository id of the repository to configure, e.g. `aether.connector.http.headers.central` for repository with id `central`.
Expand Down