Parallel invocation of DetektFacade fails spuriously

[davidburstrom]

Expected Behavior

It should be possible to run tests and other scenarios in parallel where DetectFacade is invoked.

Observed Behavior

As reported in e.g. Ozsie/detekt-maven-plugin#34, parallel execution can fail. A typical stack trace is

org.jetbrains.kotlin.com.intellij.openapi.extensions.impl.PicoPluginExtensionInitializationException: Duplicate registration for EP: org.jetbrains.kotlin.com.intellij.treeCopyHandler: original plugin null, new plugin null
        at org.jetbrains.kotlin.com.intellij.openapi.extensions.impl.ExtensionsAreaImpl.checkThatPointNotDuplicated(ExtensionsAreaImpl.java:306)
        at org.jetbrains.kotlin.com.intellij.openapi.extensions.impl.ExtensionsAreaImpl.registerExtensionPoint(ExtensionsAreaImpl.java:311)
        at org.jetbrains.kotlin.com.intellij.openapi.extensions.impl.ExtensionsAreaImpl.doRegisterExtensionPoint(ExtensionsAreaImpl.java:287)
        at org.jetbrains.kotlin.com.intellij.openapi.extensions.impl.ExtensionsAreaImpl.registerExtensionPoint(ExtensionsAreaImpl.java:260)
        at io.gitlab.arturbosch.detekt.api.internal.DetektPomModel.<init>(DetektPomModel.kt:27)
        at io.gitlab.arturbosch.detekt.api.internal.KotlinEnvironmentUtilsKt.createKotlinCoreEnvironment(KotlinEnvironmentUtils.kt:56)
        at io.gitlab.arturbosch.detekt.core.ProcessingSettings$environment$2.invoke(ProcessingSettings.kt:102)
        at io.gitlab.arturbosch.detekt.core.ProcessingSettings$environment$2.invoke(ProcessingSettings.kt:28)
        at kotlin.SynchronizedLazyImpl.getValue(LazyJVM.kt:74)
        at io.gitlab.arturbosch.detekt.core.ProcessingSettings.getEnvironment(ProcessingSettings.kt)
        at io.gitlab.arturbosch.detekt.core.DetektFacade.<init>(DetektFacade.kt:29)
        at io.gitlab.arturbosch.detekt.core.DetektFacade$Companion.create(DetektFacade.kt:107)
        at io.gitlab.arturbosch.detekt.core.DetektFacade$Companion.create(DetektFacade.kt:91)
        at org.difflint.checkers.detekt.DetektChecker.analyseFiles(DetektChecker.java:137)
        at org.difflint.checkers.detekt.DetektChecker.check(DetektChecker.java:93)
        at org.difflint.Analyzer.lambda$analyze$1(Analyzer.java:145)
        at java.util.stream.ForEachOps$ForEachOp$OfRef.accept(ForEachOps.java:184)

Steps to Reproduce

Reproduction could be provided if requested.

Context

Looking at the implementation of DetektPomModel (https://github.com/arturbosch/detekt/blob/2f7859844a8eefc05a0e58a4e551dac143a58a8e/detekt-api/src/main/kotlin/io/gitlab/arturbosch/detekt/api/internal/DetektPomModel.kt#L26) it looks like there is an attempt not to dual-register the extension point. However, this faces race conditions, where two threads may see that the singleton null/root area does not have the EP registered, and then attempts to register the EP in parallel.

Your Environment

• Version of Detekt used: 1.6.0
• Version of Gradle used (if applicable): 6.2.2
• Operating System and version: N/A

[ZacSweers]

I still see this even with 1.7.1 on gradle projects

[arturbosch]

Has someone an open repository where this is like 1/10 reliably reproducable?

[ZacSweers]

Ours is private but I'm happy to run any debugging steps if it helps

[arturbosch]

I could not reproduce a crash with current master.
When I remove the merged fix I can reproduce this.

package io.gitlab.arturbosch.detekt.core

import org.spekframework.spek2.Spek
import org.spekframework.spek2.style.specification.describe
import java.util.concurrent.Executors
import java.util.concurrent.Future

internal class ProcessingSettingsTest : Spek({

    describe("runs") {

        it("bla") {
            val service = Executors.newFixedThreadPool(4)

            val futures = mutableListOf<Future<*>>()
            for (_i in (0..1000)) {
                val future = service.submit {
                    val settings = ProcessingSettings(
                        emptyList(),
                        outPrinter = System.out,
                        errPrinter = System.err
                    )
                    val facade = DetektFacade.create(settings)
                    facade.run()
                }
                futures.add(future)
            }

            futures.forEach { it.get() }
        }
    }
})

@ZacSweers @davidburstrom please test it again with 1.7.2

[davidburstrom]

I've tried removing my workaround (external synchronization) and tried my compatibility test suite with 1.6.0, 1.7.0, 1.7.1 and 1.7.2. I was only able to reproduce the bug on 1.6.0, about 1 out of 3 runs, and never on 1.7.0+. My guess is that it's a similar but different issue for @ZacSweers?

[ZacSweers]

1.7.2 works for us 👍