Skip to content

Commit 1e12210

Browse files
committed
Validate FJP.commonPool parallelism to avoid pathological bugs
Also always construct private FJP pool if kotlinx.coroutines.default.parallelism is explicitly specified. Fixes #432, #288
1 parent b10287e commit 1e12210

File tree

1 file changed

+27
-12
lines changed

1 file changed

+27
-12
lines changed

core/kotlinx-coroutines-core/src/CommonPool.kt

+27-12
Original file line numberDiff line numberDiff line change
@@ -31,19 +31,20 @@ object CommonPool : CoroutineDispatcher() {
3131
*/
3232
public const val DEFAULT_PARALLELISM_PROPERTY_NAME = "kotlinx.coroutines.default.parallelism"
3333

34-
private val parallelism = run<Int> {
35-
val property = Try { System.getProperty(DEFAULT_PARALLELISM_PROPERTY_NAME) }
36-
if (property == null) {
37-
(Runtime.getRuntime().availableProcessors() - 1).coerceAtLeast(1)
38-
} else {
39-
val parallelism = property.toIntOrNull()
40-
if (parallelism == null || parallelism < 1) {
41-
error("Expected positive number in $DEFAULT_PARALLELISM_PROPERTY_NAME, but has $property")
42-
}
43-
parallelism
34+
// Equals to -1 if not explicitly specified
35+
private val requestedParallelism = run<Int> {
36+
val property = Try { System.getProperty(DEFAULT_PARALLELISM_PROPERTY_NAME) } ?: return@run -1
37+
val parallelism = property.toIntOrNull()
38+
if (parallelism == null || parallelism < 1) {
39+
error("Expected positive number in $DEFAULT_PARALLELISM_PROPERTY_NAME, but has $property")
4440
}
41+
parallelism
4542
}
4643

44+
private val parallelism: Int
45+
get() = requestedParallelism.takeIf { it > 0 }
46+
?: (Runtime.getRuntime().availableProcessors() - 1).coerceAtLeast(1)
47+
4748
// For debug and tests
4849
private var usePrivatePool = false
4950

@@ -54,17 +55,31 @@ object CommonPool : CoroutineDispatcher() {
5455

5556
private fun createPool(): ExecutorService {
5657
if (System.getSecurityManager() != null) return createPlainPool()
58+
// Reflection on ForkJoinPool class so that it works on JDK 6 (which is absent there)
5759
val fjpClass = Try { Class.forName("java.util.concurrent.ForkJoinPool") }
58-
?: return createPlainPool()
59-
if (!usePrivatePool) {
60+
?: return createPlainPool() // Fallback to plain thread pool
61+
// Try to use commonPool unless parallelism was explicitly specified or int debug privatePool mode
62+
if (!usePrivatePool && requestedParallelism < 0) {
6063
Try { fjpClass.getMethod("commonPool")?.invoke(null) as? ExecutorService }
64+
?.takeIf { isGoodCommonPool(fjpClass, it) }
6165
?.let { return it }
6266
}
67+
// Try to create private ForkJoinPool instance
6368
Try { fjpClass.getConstructor(Int::class.java).newInstance(parallelism) as? ExecutorService }
6469
?. let { return it }
70+
// Fallback to plain thread pool
6571
return createPlainPool()
6672
}
6773

74+
/**
75+
* Checks that this ForkJoinPool's parallelism is at least one to avoid pathological bugs.
76+
*/
77+
private fun isGoodCommonPool(fjpClass: Class<*>, executor: ExecutorService): Boolean {
78+
val actual = Try { fjpClass.getMethod("getParallelism").invoke(executor) as? Int }
79+
?: return false
80+
return actual >= 1
81+
}
82+
6883
private fun createPlainPool(): ExecutorService {
6984
val threadId = AtomicInteger()
7085
return Executors.newFixedThreadPool(parallelism) {

0 commit comments

Comments
 (0)