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

Workaround for slow localhost lookup JDK bug on macOS #11134

Merged
merged 2 commits into from Oct 28, 2019
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Expand Up @@ -24,10 +24,7 @@
import java.io.IOException;
import java.io.InputStream;
import java.io.OutputStream;
import java.net.InetAddress;
import java.net.UnknownHostException;
import java.util.Properties;
import java.util.function.Consumer;

public class OriginMetadataFactory {

Expand All @@ -43,20 +40,27 @@ public class OriginMetadataFactory {
private static final String HOST_NAME_KEY = "hostName";
private static final String USER_NAME_KEY = "userName";

private final File rootDir;
private final String userName;
private final String operatingSystem;
private final String currentBuildInvocationId;
private final Consumer<Properties> additionalProperties;
private final File rootDir;

private volatile String localHostName;
private final PropertiesConfigurator additionalProperties;
private final HostnameLookup hostnameLookup;

public OriginMetadataFactory(File rootDir, String userName, String operatingSystem, String currentBuildInvocationId, Consumer<Properties> additionalProperties) {
public OriginMetadataFactory(
File rootDir,
String userName,
String operatingSystem,
String currentBuildInvocationId,
PropertiesConfigurator additionalProperties,
HostnameLookup hostnameLookup
Copy link
Member

Choose a reason for hiding this comment

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

I can't find the class HostnameLookup. Is that missing from the branch?

Copy link
Member Author

Choose a reason for hiding this comment

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

It's down there :)

) {
this.rootDir = rootDir;
this.userName = userName;
this.operatingSystem = operatingSystem;
this.additionalProperties = additionalProperties;
this.currentBuildInvocationId = currentBuildInvocationId;
this.hostnameLookup = hostnameLookup;
}

public OriginWriter createWriter(CacheableEntity entry, long elapsedTime) {
Expand All @@ -71,25 +75,14 @@ public void execute(OutputStream outputStream) throws IOException {
properties.setProperty(EXECUTION_TIME_KEY, Long.toString(elapsedTime));
properties.setProperty(ROOT_PATH_KEY, rootDir.getAbsolutePath());
properties.setProperty(OPERATING_SYSTEM_KEY, operatingSystem);
properties.setProperty(HOST_NAME_KEY, determineHostName());
properties.setProperty(HOST_NAME_KEY, hostnameLookup.getHostname());
properties.setProperty(USER_NAME_KEY, userName);
additionalProperties.accept(properties);
additionalProperties.configure(properties);
properties.store(outputStream, "Generated origin information");
}
};
}

private String determineHostName() {
if (localHostName == null) {
try {
localHostName = InetAddress.getLocalHost().getHostName();
} catch (UnknownHostException e) {
localHostName = "<unknown>";
}
}
return localHostName;
}

public OriginReader createReader(CacheableEntity entry) {
return new OriginReader() {
@Override
Expand All @@ -112,4 +105,12 @@ public OriginMetadata execute(InputStream inputStream) throws IOException {
}
};
}

public interface PropertiesConfigurator {
void configure(Properties properties);
}

public interface HostnameLookup {
String getHostname();
}
}
Expand Up @@ -23,7 +23,14 @@ class OriginMetadataFactoryTest extends Specification {
def entry = Mock(CacheableEntity)
def rootDir = Mock(File)
def buildInvocationId = UUID.randomUUID().toString()
def factory = new OriginMetadataFactory(rootDir, "user", "os", buildInvocationId, { it.gradleVersion = "3.0" })
def factory = new OriginMetadataFactory(
rootDir,
"user",
"os",
buildInvocationId,
{ it.gradleVersion = "3.0" },
{ "my-host" }
)

def "converts to origin metadata"() {
entry.identity >> "identity"
Expand All @@ -46,7 +53,7 @@ class OriginMetadataFactoryTest extends Specification {
origin.executionTime == "10"
origin.rootPath == "root"
origin.operatingSystem == "os"
origin.hostName == InetAddress.localHost.hostName
origin.hostName == "my-host"
origin.userName == "user"
origin.buildInvocationId == buildInvocationId
}
Expand Down
Expand Up @@ -37,6 +37,7 @@
import org.gradle.internal.nativeplatform.filesystem.FileSystem;
import org.gradle.internal.operations.BuildOperationExecutor;
import org.gradle.internal.os.OperatingSystem;
import org.gradle.internal.remote.internal.inet.InetAddressFactory;
import org.gradle.internal.scopeids.id.BuildInvocationScopeId;
import org.gradle.internal.service.ServiceRegistry;
import org.gradle.internal.snapshot.FileSystemMirror;
Expand All @@ -60,8 +61,9 @@ BuildCacheEntryPacker createResultPacker(FileSystem fileSystem, StreamHasher fil
}

OriginMetadataFactory createOriginMetadataFactory(
BuildInvocationScopeId buildInvocationScopeId,
GradleInternal gradleInternal,
BuildInvocationScopeId buildInvocationScopeId
InetAddressFactory inetAddressFactory
) {
File rootDir = gradleInternal.getRootProject().getRootDir();
return new OriginMetadataFactory(
Expand All @@ -71,7 +73,8 @@ OriginMetadataFactory createOriginMetadataFactory(
buildInvocationScopeId.getId().asString(),
properties -> {
properties.setProperty(GRADLE_VERSION_KEY, GradleVersion.current().getVersion());
}
},
inetAddressFactory::getHostname
);
}

Expand Down
Expand Up @@ -15,9 +15,13 @@
*/
package org.gradle.internal.remote.internal.inet;

import org.gradle.internal.os.OperatingSystem;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import javax.annotation.Nullable;
import java.io.BufferedReader;
import java.io.InputStreamReader;
import java.net.InetAddress;
import java.net.InetSocketAddress;
import java.net.UnknownHostException;
Expand All @@ -37,16 +41,41 @@ public class InetAddressFactory {
private String hostName;

public String getHostname() {
synchronized (lock) {
if (hostName == null) {
try {
hostName = InetAddress.getLocalHost().getHostName();
} catch (UnknownHostException e) {
hostName = getCommunicationAddresses().get(0).toString();
if (hostName == null) {
synchronized (lock) {
// Work around https://bugs.openjdk.java.net/browse/JDK-8143378 on macOS
// See also https://stackoverflow.com/a/39698914
if (hostName == null && OperatingSystem.current() == OperatingSystem.MAC_OS) {
ProcessBuilder builder = new ProcessBuilder("hostname");
try {
Process process = builder.start();
int result = process.waitFor();
if (result == 0) {
BufferedReader reader = new BufferedReader(new InputStreamReader(process.getInputStream()));
try {
String line = reader.readLine();
if (line != null) {
hostName = line;
}
} finally {
reader.close();
}
}
} catch (Exception e) {
logger.debug("Couldn't resolve hostname by running hostname, falling back to using JDK", e);
}
}

if (hostName == null) {
try {
hostName = InetAddress.getLocalHost().getHostName();
} catch (UnknownHostException e) {
hostName = getCommunicationAddresses().get(0).toString();
}
}
}
return hostName;
}
return hostName;
}

/**
Expand Down Expand Up @@ -114,7 +143,7 @@ private void handleOpenshift() {
}
}


@Nullable
private InetAddress findOpenshiftAddresses() {
for (String key : System.getenv().keySet()) {
if (key.startsWith("OPENSHIFT_") && key.endsWith("_IP")) {
Expand Down
@@ -0,0 +1,19 @@
/*
* Copyright 2019 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
@NonNullApi
package org.gradle.internal.remote.internal.inet;

import org.gradle.api.NonNullApi;