Skip to content

Commit 6e0c5b5

Browse files
sfc-gh-ebrossardcpovirk
authored andcommitted
Make AbstractFuture compatible with ForkJoinPool by catching exceptions from property retrieval.
Fixes #3788, #3784 RELNOTES=Made it safe to load the `AbstractFuture` class from a `ForkJoinPool` thread under a security manager. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=293696683
1 parent c6f48dc commit 6e0c5b5

File tree

3 files changed

+156
-6
lines changed

3 files changed

+156
-6
lines changed

android/guava/src/com/google/common/util/concurrent/AbstractFuture.java

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -69,9 +69,20 @@ public abstract class AbstractFuture<V> extends InternalFutureFailureAccess
6969
implements ListenableFuture<V> {
7070
// NOTE: Whenever both tests are cheap and functional, it's faster to use &, | instead of &&, ||
7171

72-
private static final boolean GENERATE_CANCELLATION_CAUSES =
73-
Boolean.parseBoolean(
74-
System.getProperty("guava.concurrent.generate_cancellation_cause", "false"));
72+
private static final boolean GENERATE_CANCELLATION_CAUSES;
73+
74+
static {
75+
// System.getProperty may throw if the security policy does not permit access.
76+
boolean generateCancellationCauses;
77+
try {
78+
generateCancellationCauses =
79+
Boolean.parseBoolean(
80+
System.getProperty("guava.concurrent.generate_cancellation_cause", "false"));
81+
} catch (SecurityException e) {
82+
generateCancellationCauses = false;
83+
}
84+
GENERATE_CANCELLATION_CAUSES = generateCancellationCauses;
85+
}
7586

7687
/**
7788
* Tag interface marking trusted subclasses. This enables some optimizations. The implementation
Lines changed: 128 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,128 @@
1+
/*
2+
* Copyright (C) 2020 The Guava Authors
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package com.google.common.util.concurrent;
18+
19+
import java.net.URLClassLoader;
20+
import java.security.Permission;
21+
import java.util.HashMap;
22+
import java.util.Map;
23+
import java.util.PropertyPermission;
24+
import java.util.concurrent.CountDownLatch;
25+
import java.util.concurrent.ForkJoinPool;
26+
import java.util.concurrent.TimeUnit;
27+
import javax.annotation.concurrent.GuardedBy;
28+
import junit.framework.TestCase;
29+
30+
/** Tests for {@link AbstractFuture} using an innocuous thread. */
31+
32+
public class AbstractFutureInnocuousThreadTest extends TestCase {
33+
private ClassLoader oldClassLoader;
34+
private URLClassLoader classReloader;
35+
private Class<?> settableFutureClass;
36+
private SecurityManager oldSecurityManager;
37+
38+
@Override
39+
protected void setUp() throws Exception {
40+
// Load the "normal" copy of SettableFuture and related classes.
41+
SettableFuture<?> unused = SettableFuture.create();
42+
// Hack to load AbstractFuture et. al. in a new classloader so that it tries to re-read the
43+
// cancellation-cause system property. This allows us to test what happens if reading the
44+
// property is forbidden and then continue running tests normally in one jvm without resorting
45+
// to even crazier hacks to reset static final boolean fields.
46+
final String concurrentPackage = SettableFuture.class.getPackage().getName();
47+
classReloader =
48+
new URLClassLoader(ClassPathUtil.getClassPathUrls()) {
49+
@GuardedBy("loadedClasses")
50+
final Map<String, Class<?>> loadedClasses = new HashMap<>();
51+
52+
@Override
53+
public Class<?> loadClass(String name) throws ClassNotFoundException {
54+
if (name.startsWith(concurrentPackage)
55+
// Use other classloader for ListenableFuture, so that the objects can interact
56+
&& !ListenableFuture.class.getName().equals(name)) {
57+
synchronized (loadedClasses) {
58+
Class<?> toReturn = loadedClasses.get(name);
59+
if (toReturn == null) {
60+
toReturn = super.findClass(name);
61+
loadedClasses.put(name, toReturn);
62+
}
63+
return toReturn;
64+
}
65+
}
66+
return super.loadClass(name);
67+
}
68+
};
69+
oldClassLoader = Thread.currentThread().getContextClassLoader();
70+
Thread.currentThread().setContextClassLoader(classReloader);
71+
72+
oldSecurityManager = System.getSecurityManager();
73+
/*
74+
* TODO(cpovirk): Why couldn't I get this to work with PermissionCollection and implies(), as
75+
* used by ClassPathTest?
76+
*/
77+
final PropertyPermission readSystemProperty =
78+
new PropertyPermission("guava.concurrent.generate_cancellation_cause", "read");
79+
SecurityManager disallowPropertySecurityManager =
80+
new SecurityManager() {
81+
@Override
82+
public void checkPermission(Permission p) {
83+
if (readSystemProperty.equals(p)) {
84+
throw new SecurityException("Disallowed: " + p);
85+
}
86+
}
87+
};
88+
System.setSecurityManager(disallowPropertySecurityManager);
89+
90+
settableFutureClass = classReloader.loadClass(SettableFuture.class.getName());
91+
92+
/*
93+
* We must keep the SecurityManager installed during the test body: It affects what kind of
94+
* threads ForkJoinPool.commonPool() creates.
95+
*/
96+
}
97+
98+
@Override
99+
protected void tearDown() throws Exception {
100+
System.setSecurityManager(oldSecurityManager);
101+
classReloader.close();
102+
Thread.currentThread().setContextClassLoader(oldClassLoader);
103+
}
104+
105+
public void testAbstractFutureInitializationWithInnocuousThread_doesNotThrow() throws Exception {
106+
CountDownLatch latch = new CountDownLatch(1);
107+
// Setting a security manager causes the common ForkJoinPool to use InnocuousThreads with no
108+
// permissions.
109+
// submit()/join() causes this thread to execute the task instead, so we use a CountDownLatch as
110+
// a barrier to synchronize.
111+
// TODO(cpovirk): If some other test already initialized commonPool(), this won't work :(
112+
// Maybe we should just run this test in its own VM.
113+
ForkJoinPool.commonPool()
114+
.execute(
115+
() -> {
116+
try {
117+
settableFutureClass.getMethod("create").invoke(null);
118+
latch.countDown();
119+
} catch (Exception e) {
120+
throw new RuntimeException(e);
121+
}
122+
});
123+
// In the failure case, await() will timeout.
124+
assertTrue(latch.await(2, TimeUnit.SECONDS));
125+
}
126+
127+
// TODO(cpovirk): Write a similar test that doesn't use ForkJoinPool (to run under Android)?
128+
}

guava/src/com/google/common/util/concurrent/AbstractFuture.java

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -69,9 +69,20 @@ public abstract class AbstractFuture<V> extends InternalFutureFailureAccess
6969
implements ListenableFuture<V> {
7070
// NOTE: Whenever both tests are cheap and functional, it's faster to use &, | instead of &&, ||
7171

72-
private static final boolean GENERATE_CANCELLATION_CAUSES =
73-
Boolean.parseBoolean(
74-
System.getProperty("guava.concurrent.generate_cancellation_cause", "false"));
72+
private static final boolean GENERATE_CANCELLATION_CAUSES;
73+
74+
static {
75+
// System.getProperty may throw if the security policy does not permit access.
76+
boolean generateCancellationCauses;
77+
try {
78+
generateCancellationCauses =
79+
Boolean.parseBoolean(
80+
System.getProperty("guava.concurrent.generate_cancellation_cause", "false"));
81+
} catch (SecurityException e) {
82+
generateCancellationCauses = false;
83+
}
84+
GENERATE_CANCELLATION_CAUSES = generateCancellationCauses;
85+
}
7586

7687
/**
7788
* Tag interface marking trusted subclasses. This enables some optimizations. The implementation

0 commit comments

Comments
 (0)