Skip to content

Commit f52b4e5

Browse files
author
Louis Ryan
committed
Make the OkHTTP transport AppEngine friendly. AppEngine may support
conscrypt at some point which would allow ALPN to function Clarify the SSLContext.getDefault is not used when constructing the default SSLSocketFactory.
1 parent d2cc576 commit f52b4e5

File tree

8 files changed

+156
-56
lines changed

8 files changed

+156
-56
lines changed

core/src/main/java/io/grpc/internal/GrpcUtil.java

Lines changed: 32 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@
3939
import com.google.common.base.Stopwatch;
4040
import com.google.common.base.Supplier;
4141
import com.google.common.collect.ImmutableMap;
42+
import com.google.common.util.concurrent.MoreExecutors;
4243
import com.google.common.util.concurrent.ThreadFactoryBuilder;
4344

4445
import io.grpc.Metadata;
@@ -53,6 +54,7 @@
5354
import java.util.concurrent.ExecutorService;
5455
import java.util.concurrent.Executors;
5556
import java.util.concurrent.ScheduledExecutorService;
57+
import java.util.concurrent.ThreadFactory;
5658
import java.util.concurrent.TimeUnit;
5759

5860
import javax.annotation.Nullable;
@@ -62,6 +64,12 @@
6264
*/
6365
public final class GrpcUtil {
6466

67+
// Certain production AppEngine runtimes have constraints on threading and socket handling
68+
// that need to be accommodated.
69+
public static final boolean IS_RESTRICTED_APPENGINE =
70+
"Production".equals(System.getProperty("com.google.appengine.runtime.environment"))
71+
&& "1.7".equals(System.getProperty("java.specification.version"));
72+
6573
/**
6674
* {@link io.grpc.Metadata.Key} for the timeout header.
6775
*/
@@ -374,10 +382,7 @@ public static String authorityFromHostAndPort(String host, int port) {
374382
private static final String name = "grpc-default-executor";
375383
@Override
376384
public ExecutorService create() {
377-
return Executors.newCachedThreadPool(new ThreadFactoryBuilder()
378-
.setDaemon(true)
379-
.setNameFormat(name + "-%d")
380-
.build());
385+
return Executors.newCachedThreadPool(getThreadFactory(name + "-%d", true));
381386
}
382387

383388
@Override
@@ -402,10 +407,8 @@ public ScheduledExecutorService create() {
402407
// ScheduledThreadPoolExecutor.
403408
ScheduledExecutorService service = Executors.newScheduledThreadPool(
404409
1,
405-
new ThreadFactoryBuilder()
406-
.setDaemon(true)
407-
.setNameFormat("grpc-timer-%d")
408-
.build());
410+
getThreadFactory("grpc-timer-%d", true));
411+
409412
// If there are long timeouts that are cancelled, they will not actually be removed from
410413
// the executors queue. This forces immediate removal upon cancellation to avoid a
411414
// memory leak. Reflection is used because we cannot use methods added in Java 1.7. If
@@ -431,6 +434,27 @@ public void close(ScheduledExecutorService instance) {
431434
}
432435
};
433436

437+
438+
/**
439+
* Get a {@link ThreadFactory} suitable for use in the current environment.
440+
* @param nameFormat to apply to threads created by the factory.
441+
* @param daemon {@code true} if the threads the factory creates are daemon threads, {@code false}
442+
* otherwise.
443+
* @return a {@link ThreadFactory}.
444+
*/
445+
public static ThreadFactory getThreadFactory(String nameFormat, boolean daemon) {
446+
ThreadFactory threadFactory = MoreExecutors.platformThreadFactory();
447+
if (IS_RESTRICTED_APPENGINE) {
448+
return threadFactory;
449+
} else {
450+
return new ThreadFactoryBuilder()
451+
.setThreadFactory(threadFactory)
452+
.setDaemon(daemon)
453+
.setNameFormat(nameFormat)
454+
.build();
455+
}
456+
}
457+
434458
/**
435459
* The factory of default Stopwatches.
436460
*/

core/src/main/java/io/grpc/internal/SharedResourceHolder.java

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,6 @@
3232
package io.grpc.internal;
3333

3434
import com.google.common.base.Preconditions;
35-
import com.google.common.util.concurrent.ThreadFactoryBuilder;
3635

3736
import java.util.IdentityHashMap;
3837
import java.util.concurrent.Executors;
@@ -65,10 +64,8 @@ public final class SharedResourceHolder {
6564
new ScheduledExecutorFactory() {
6665
@Override
6766
public ScheduledExecutorService createScheduledExecutor() {
68-
return Executors.newSingleThreadScheduledExecutor(new ThreadFactoryBuilder()
69-
.setDaemon(true)
70-
.setNameFormat("grpc-shared-destroyer-%d")
71-
.build());
67+
return Executors.newSingleThreadScheduledExecutor(
68+
GrpcUtil.getThreadFactory("grpc-shared-destroyer-%d", true));
7269
}
7370
});
7471

interop-testing/src/test/java/io/grpc/testing/integration/Http2OkHttpTest.java

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@
4545
import io.grpc.netty.GrpcSslContexts;
4646
import io.grpc.netty.NettyServerBuilder;
4747
import io.grpc.okhttp.OkHttpChannelBuilder;
48+
import io.grpc.okhttp.internal.Platform;
4849
import io.grpc.stub.StreamObserver;
4950
import io.grpc.testing.StreamRecorder;
5051
import io.grpc.testing.TestUtils;
@@ -60,6 +61,7 @@
6061
import org.junit.runner.RunWith;
6162
import org.junit.runners.JUnit4;
6263

64+
import java.io.FileInputStream;
6365
import java.io.IOException;
6466

6567
import javax.net.ssl.SSLPeerUnverifiedException;
@@ -109,7 +111,8 @@ protected ManagedChannel createChannel() {
109111
.overrideAuthority(GrpcUtil.authorityFromHostAndPort(
110112
TestUtils.TEST_SERVER_HOST, getPort()));
111113
try {
112-
builder.sslSocketFactory(TestUtils.newSslSocketFactoryForCa(TestUtils.loadCert("ca.pem")));
114+
builder.sslSocketFactory(TestUtils.newSslSocketFactoryForCa(Platform.get().getProvider(),
115+
new FileInputStream(TestUtils.loadCert("ca.pem"))));
113116
} catch (Exception e) {
114117
throw new RuntimeException(e);
115118
}
@@ -149,7 +152,8 @@ public void wrongHostNameFailHostnameVerification() throws Exception {
149152
.overrideAuthority(GrpcUtil.authorityFromHostAndPort(
150153
"I.am.a.bad.hostname", getPort()));
151154
ManagedChannel channel = builder.sslSocketFactory(
152-
TestUtils.newSslSocketFactoryForCa(TestUtils.loadCert("ca.pem"))).build();
155+
TestUtils.newSslSocketFactoryForCa(Platform.get().getProvider(),
156+
new FileInputStream(TestUtils.loadCert("ca.pem")))).build();
153157
TestServiceGrpc.TestServiceBlockingStub blockingStub =
154158
TestServiceGrpc.newBlockingStub(channel);
155159

okhttp/src/main/java/io/grpc/okhttp/OkHttpChannelBuilder.java

Lines changed: 21 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,6 @@
3838

3939
import com.google.common.annotations.VisibleForTesting;
4040
import com.google.common.base.Preconditions;
41-
import com.google.common.util.concurrent.ThreadFactoryBuilder;
4241

4342
import com.squareup.okhttp.CipherSuite;
4443
import com.squareup.okhttp.ConnectionSpec;
@@ -54,15 +53,18 @@
5453
import io.grpc.internal.GrpcUtil;
5554
import io.grpc.internal.SharedResourceHolder;
5655
import io.grpc.internal.SharedResourceHolder.Resource;
56+
import io.grpc.okhttp.internal.Platform;
5757

5858
import java.net.InetSocketAddress;
5959
import java.net.SocketAddress;
60+
import java.security.GeneralSecurityException;
6061
import java.util.concurrent.Executor;
6162
import java.util.concurrent.ExecutorService;
6263
import java.util.concurrent.Executors;
6364
import java.util.concurrent.TimeUnit;
6465

6566
import javax.annotation.Nullable;
67+
import javax.net.ssl.SSLContext;
6668
import javax.net.ssl.SSLSocketFactory;
6769

6870
/** Convenience class for building channels with the OkHttp transport. */
@@ -90,10 +92,7 @@ public class OkHttpChannelBuilder extends
9092
new Resource<ExecutorService>() {
9193
@Override
9294
public ExecutorService create() {
93-
return Executors.newCachedThreadPool(new ThreadFactoryBuilder()
94-
.setDaemon(true)
95-
.setNameFormat("grpc-okhttp-%d")
96-
.build());
95+
return Executors.newCachedThreadPool(GrpcUtil.getThreadFactory("grpc-okhttp-%d", true));
9796
}
9897

9998
@Override
@@ -147,6 +146,11 @@ public final OkHttpChannelBuilder transportExecutor(@Nullable Executor transport
147146
/**
148147
* Sets the negotiation type for the HTTP/2 connection.
149148
*
149+
* <p>If TLS is enabled a default {@link SSLSocketFactory} is created using the best
150+
* {@link java.security.Provider} available and is NOT based on
151+
* {@link SSLSocketFactory#getDefault}. To more precisely control the TLS configuration call
152+
* {@link #sslSocketFactory} to override the socket factory used.
153+
*
150154
* <p>Default: <code>TLS</code>
151155
*/
152156
public final OkHttpChannelBuilder negotiationType(NegotiationType type) {
@@ -180,7 +184,8 @@ public final OkHttpChannelBuilder enableKeepAlive(boolean enable, long keepAlive
180184
}
181185

182186
/**
183-
* Provides a SSLSocketFactory to replace the default SSLSocketFactory used for TLS.
187+
* Override the default {@link SSLSocketFactory} and enable {@link NegotiationType#TLS}
188+
* negotiation.
184189
*
185190
* <p>By default, when TLS is enabled, <code>SSLSocketFactory.getDefault()</code> will be used.
186191
*
@@ -264,8 +269,16 @@ protected Attributes getNameResolverParams() {
264269
SSLSocketFactory createSocketFactory() {
265270
switch (negotiationType) {
266271
case TLS:
267-
return sslSocketFactory == null
268-
? (SSLSocketFactory) SSLSocketFactory.getDefault() : sslSocketFactory;
272+
try {
273+
if (sslSocketFactory == null) {
274+
SSLContext sslContext = SSLContext.getInstance("TLS", Platform.get().getProvider());
275+
sslContext.init(null, null, null);
276+
sslSocketFactory = sslContext.getSocketFactory();
277+
}
278+
return sslSocketFactory;
279+
} catch (GeneralSecurityException gse) {
280+
throw new RuntimeException("TLS Provider failure", gse);
281+
}
269282
case PLAINTEXT:
270283
return null;
271284
default:

okhttp/src/main/java/io/grpc/okhttp/OkHttpChannelProvider.java

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,18 +33,22 @@
3333

3434
import io.grpc.Internal;
3535
import io.grpc.ManagedChannelProvider;
36+
import io.grpc.internal.GrpcUtil;
3637

37-
/** Provider for {@link OkHttpChannelBuilder} instances. */
38+
/**
39+
* Provider for {@link OkHttpChannelBuilder} instances.
40+
*/
3841
@Internal
3942
public final class OkHttpChannelProvider extends ManagedChannelProvider {
43+
4044
@Override
4145
public boolean isAvailable() {
4246
return true;
4347
}
4448

4549
@Override
4650
public int priority() {
47-
return isAndroid() ? 8 : 3;
51+
return (GrpcUtil.IS_RESTRICTED_APPENGINE || isAndroid()) ? 8 : 3;
4852
}
4953

5054
@Override

okhttp/src/main/java/io/grpc/okhttp/OkHttpClientTransport.java

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -735,7 +735,9 @@ class ClientFrameHandler implements FrameReader.Handler, Runnable {
735735
@Override
736736
public void run() {
737737
String threadName = Thread.currentThread().getName();
738-
Thread.currentThread().setName("OkHttpClientTransport");
738+
if (!GrpcUtil.IS_RESTRICTED_APPENGINE) {
739+
Thread.currentThread().setName("OkHttpClientTransport");
740+
}
739741
try {
740742
// Read until the underlying socket closes.
741743
while (frameReader.nextFrame(this)) {
@@ -758,8 +760,10 @@ public void run() {
758760
log.log(Level.INFO, "Exception closing frame reader", ex);
759761
}
760762
listener.transportTerminated();
761-
// Restore the original thread name.
762-
Thread.currentThread().setName(threadName);
763+
if (!GrpcUtil.IS_RESTRICTED_APPENGINE) {
764+
// Restore the original thread name.
765+
Thread.currentThread().setName(threadName);
766+
}
763767
}
764768
}
765769

0 commit comments

Comments
 (0)