Fix concurrency issue loading RuleSetProviders (#102)

Addresses #101 The iterator returned by `ServiceLoader.load()` lazily resolves classes from the classpath and is *not* safe to use in the gradle workers.
jeremymailen · May 17, 2019 · 41bf9f2 · 41bf9f2
1 parent eca64ee
commit 41bf9f2
Show file tree

Hide file tree

Showing 6 changed files with 15 additions and 14 deletions.
diff --git a/README.md b/README.md
@@ -15,7 +15,7 @@ Available on the Gradle Plugins Portal: https://plugins.gradle.org/plugin/org.jm
 
 ```kotlin
 plugins {
-    id("org.jmailen.kotlinter") version "1.25.1"
+    id("org.jmailen.kotlinter") version "1.25.2"
 }
 ```
 
@@ -26,7 +26,7 @@ plugins {
 
 ```groovy
 plugins {
-    id "org.jmailen.kotlinter" version "1.25.1"
+    id "org.jmailen.kotlinter" version "1.25.2"
 }
 ```
 
@@ -46,7 +46,7 @@ buildscript {
         }
     }
     dependencies {
-        classpath("org.jmailen.gradle:kotlinter-gradle:1.25.1")
+        classpath("org.jmailen.gradle:kotlinter-gradle:1.25.2")
     }
 }
 ```
@@ -71,7 +71,7 @@ buildscript {
         }
     }
     dependencies {
-        classpath "org.jmailen.gradle:kotlinter-gradle:1.25.1"
+        classpath "org.jmailen.gradle:kotlinter-gradle:1.25.2"
     }
 }
 ```

diff --git a/build.gradle b/build.gradle
@@ -3,7 +3,7 @@ plugins {
     id 'com.gradle.plugin-publish' version '0.10.0'
     id 'java-gradle-plugin'
     id 'maven-publish'
-    id 'org.jmailen.kotlinter' version '1.25.0'
+    id 'org.jmailen.kotlinter' version '1.25.1'
     id 'idea'
 }
 
@@ -30,7 +30,7 @@ tasks.withType(PluginUnderTestMetadata).configureEach {
     pluginClasspath.from(configurations.compileOnly)
 }
 
-version = '1.25.1'
+version = '1.25.2'
 group = 'org.jmailen.gradle'
 def pluginId = 'org.jmailen.kotlinter'
 

diff --git a/src/main/kotlin/org/jmailen/gradle/kotlinter/support/ruleSets.kt b/src/main/kotlin/org/jmailen/gradle/kotlinter/support/ruleSets.kt
@@ -21,4 +21,5 @@ fun resolveRuleSets(
         })
 }
 
-fun defaultProviders() = ServiceLoader.load(RuleSetProvider::class.java)
+// statically resolve providers from plugin classpath
+val defaultRuleSetProviders = ServiceLoader.load(RuleSetProvider::class.java).map { it }
diff --git a/src/main/kotlin/org/jmailen/gradle/kotlinter/tasks/FormatTask.kt b/src/main/kotlin/org/jmailen/gradle/kotlinter/tasks/FormatTask.kt
@@ -7,7 +7,7 @@ import org.gradle.api.tasks.TaskAction
 import org.gradle.workers.WorkerExecutor
 import org.jmailen.gradle.kotlinter.KotlinterExtension
 import org.jmailen.gradle.kotlinter.support.ExecutionContextRepository
-import org.jmailen.gradle.kotlinter.support.defaultProviders
+import org.jmailen.gradle.kotlinter.support.defaultRuleSetProviders
 import org.jmailen.gradle.kotlinter.tasks.format.FormatExecutionContext
 import org.jmailen.gradle.kotlinter.tasks.format.FormatWorkerConfigurationAction
 import org.jmailen.gradle.kotlinter.tasks.format.FormatWorkerParameters
@@ -41,7 +41,7 @@ open class FormatTask @Inject constructor(
     @TaskAction
     fun run() {
         val executionContextRepository = ExecutionContextRepository.formatInstance
-        val executionContext = FormatExecutionContext(defaultProviders(), logger)
+        val executionContext = FormatExecutionContext(defaultRuleSetProviders, logger)
         val executionContextRepositoryId = executionContextRepository.register(executionContext)
 
         source

diff --git a/src/main/kotlin/org/jmailen/gradle/kotlinter/tasks/LintTask.kt b/src/main/kotlin/org/jmailen/gradle/kotlinter/tasks/LintTask.kt
@@ -14,7 +14,7 @@ import org.gradle.workers.WorkerExecutor
 import org.jmailen.gradle.kotlinter.KotlinterExtension
 import org.jmailen.gradle.kotlinter.support.HasErrorReporter
 import org.jmailen.gradle.kotlinter.support.ExecutionContextRepository
-import org.jmailen.gradle.kotlinter.support.defaultProviders
+import org.jmailen.gradle.kotlinter.support.defaultRuleSetProviders
 import org.jmailen.gradle.kotlinter.support.reporterFor
 import org.jmailen.gradle.kotlinter.tasks.lint.LintExecutionContext
 import org.jmailen.gradle.kotlinter.tasks.lint.LintWorkerConfigurationAction
@@ -61,7 +61,7 @@ open class LintTask @Inject constructor(
             reporterFor(reporter, report)
         } + hasErrorReporter
         val executionContextRepository = ExecutionContextRepository.lintInstance
-        val executionContextRepositoryId = executionContextRepository.register(LintExecutionContext(defaultProviders(), reporters, logger))
+        val executionContextRepositoryId = executionContextRepository.register(LintExecutionContext(defaultRuleSetProviders, reporters, logger))
 
         reporters.onEach { it.beforeAll() }
 

diff --git a/src/test/kotlin/org/jmailen/gradle/kotlinter/support/RuleSetsTest.kt b/src/test/kotlin/org/jmailen/gradle/kotlinter/support/RuleSetsTest.kt
@@ -13,14 +13,14 @@ class RuleSetsTest {
 
     @Test
     fun `resolveRuleSets loads from classpath providers`() {
-        val result = resolveRuleSets(defaultProviders())
+        val result = resolveRuleSets(defaultRuleSetProviders)
 
         assertEquals(listOf("standard"), result.map { it.id })
     }
 
     @Test
     fun `resolveRuleSets loads from classpath providers including experimental rules`() {
-        val result = resolveRuleSets(defaultProviders(), true)
+        val result = resolveRuleSets(defaultRuleSetProviders, true)
 
         assertEquals(listOf("standard", "experimental"), result.map { it.id })
     }
@@ -40,7 +40,7 @@ class RuleSetsTest {
 
     @Test
     fun `test compatibility`() {
-        KtLint.lint("""fun someFunc() = """"", resolveRuleSets(defaultProviders())) {}
+        KtLint.lint("""fun someFunc() = """"", resolveRuleSets(defaultRuleSetProviders)) {}
     }
 }