From 4db9174b4d890707be69956894b86cc05ff4e222 Mon Sep 17 00:00:00 2001 From: Nikita Katkov Date: Mon, 10 Dec 2018 18:15:14 +0300 Subject: [PATCH] Error on calling suspend functions inside Lock.withLock body See SuspensionPointInsideCriticalSectionChecker --- ...=> SuspensionPointInsideMutexLockChecker.kt} | 14 ++++++++++++-- .../diagnostics/DefaultErrorMessagesJvm.java | 2 +- .../resolve/jvm/diagnostics/ErrorsJvm.java | 2 +- .../jvm/platform/JvmPlatformConfigurator.kt | 2 +- .../cli/jvm/suspensionPointInMonitor.out | 4 ++-- .../coroutines/suspensionPointInMonitor.kt | 16 ++++++++++------ .../suspensionPointInMonitorNewInf.kt | 17 +++++++++++------ 7 files changed, 38 insertions(+), 19 deletions(-) rename compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/checkers/{SuspensionPointInSynchronizedCallChecker.kt => SuspensionPointInsideMutexLockChecker.kt} (83%) diff --git a/compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/checkers/SuspensionPointInSynchronizedCallChecker.kt b/compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/checkers/SuspensionPointInsideMutexLockChecker.kt similarity index 83% rename from compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/checkers/SuspensionPointInSynchronizedCallChecker.kt rename to compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/checkers/SuspensionPointInsideMutexLockChecker.kt index 8fe690047ee..a9babe055b8 100644 --- a/compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/checkers/SuspensionPointInSynchronizedCallChecker.kt +++ b/compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/checkers/SuspensionPointInsideMutexLockChecker.kt @@ -20,7 +20,7 @@ import org.jetbrains.kotlin.resolve.calls.model.ResolvedCall import org.jetbrains.kotlin.resolve.jvm.diagnostics.ErrorsJvm import org.jetbrains.kotlin.resolve.source.getPsi -class SuspensionPointInSynchronizedCallChecker : CallChecker { +class SuspensionPointInsideMutexLockChecker : CallChecker { override fun check(resolvedCall: ResolvedCall<*>, reportOn: PsiElement, context: CallCheckerContext) { val descriptor = resolvedCall.candidateDescriptor if (descriptor !is FunctionDescriptor || !descriptor.isSuspend) return @@ -61,10 +61,20 @@ class SuspensionPointInSynchronizedCallChecker : CallChecker { if (isSynchronized) { val isSecondArgument = (resolved.valueArgumentsByIndex?.get(1) as? ExpressionValueArgument)?.valueArgument == child if (insideLambda && isSecondArgument) { - context.trace.report(ErrorsJvm.SUSPENSION_POINT_INSIDE_SYNCHRONIZED.on(reportOn, resolvedCall.resultingDescriptor)) + reportProblem(context, reportOn, resolvedCall) } return true } + + val isWithLock = resolved.resultingDescriptor.isTopLevelInPackage("withLock", "kotlin.concurrent") + if (isWithLock) { + reportProblem(context, reportOn, resolvedCall) + return true + } return false } + + private fun reportProblem(context: CallCheckerContext, reportOn: PsiElement, resolvedCall: ResolvedCall<*>) { + context.trace.report(ErrorsJvm.SUSPENSION_POINT_INSIDE_CRITICAL_SECTION.on(reportOn, resolvedCall.resultingDescriptor)) + } } \ No newline at end of file diff --git a/compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/diagnostics/DefaultErrorMessagesJvm.java b/compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/diagnostics/DefaultErrorMessagesJvm.java index d6dbaebd639..44e4a7808e4 100644 --- a/compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/diagnostics/DefaultErrorMessagesJvm.java +++ b/compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/diagnostics/DefaultErrorMessagesJvm.java @@ -139,7 +139,7 @@ public class DefaultErrorMessagesJvm implements DefaultErrorMessages.Extension { MAP.put(NON_JVM_DEFAULT_OVERRIDES_JAVA_DEFAULT, "Non-@JvmDefault interface method cannot override default Java method. Please annotate this method with @JvmDefault"); MAP.put(EXPLICIT_METADATA_IS_DISALLOWED, "Explicit @Metadata is disallowed"); MAP.put(SUSPENSION_POINT_INSIDE_MONITOR, "A suspension point at {0} is inside a critical section", STRING); - MAP.put(SUSPENSION_POINT_INSIDE_SYNCHRONIZED, "The ''{0}'' suspension point is inside a synchronized block", NAME); + MAP.put(SUSPENSION_POINT_INSIDE_CRITICAL_SECTION, "The ''{0}'' suspension point is inside a critical section", NAME); String MESSAGE_FOR_CONCURRENT_HASH_MAP_CONTAINS = "Method 'contains' from ConcurrentHashMap may have unexpected semantics: it calls 'containsValue' instead of 'containsKey'. " + diff --git a/compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/diagnostics/ErrorsJvm.java b/compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/diagnostics/ErrorsJvm.java index 70606807577..e14470ef8b7 100644 --- a/compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/diagnostics/ErrorsJvm.java +++ b/compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/diagnostics/ErrorsJvm.java @@ -126,7 +126,7 @@ public interface ErrorsJvm { DiagnosticFactory1 ANNOTATION_TARGETS_NON_EXISTENT_ACCESSOR = DiagnosticFactory1.create(WARNING); DiagnosticFactory1 SUSPENSION_POINT_INSIDE_MONITOR = DiagnosticFactory1.create(ERROR); - DiagnosticFactory1 SUSPENSION_POINT_INSIDE_SYNCHRONIZED = DiagnosticFactory1.create(ERROR); + DiagnosticFactory1 SUSPENSION_POINT_INSIDE_CRITICAL_SECTION = DiagnosticFactory1.create(ERROR); DiagnosticFactory0 CONCURRENT_HASH_MAP_CONTAINS_OPERATOR = DiagnosticFactory0.create(WARNING); DiagnosticFactory0 CONCURRENT_HASH_MAP_CONTAINS_OPERATOR_ERROR = DiagnosticFactory0.create(ERROR); diff --git a/compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/platform/JvmPlatformConfigurator.kt b/compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/platform/JvmPlatformConfigurator.kt index b96e038c291..a4bdffe4944 100644 --- a/compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/platform/JvmPlatformConfigurator.kt +++ b/compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/platform/JvmPlatformConfigurator.kt @@ -42,7 +42,7 @@ object JvmPlatformConfigurator : PlatformConfiguratorBase( additionalCallCheckers = listOf( JavaAnnotationCallChecker(), - SuspensionPointInSynchronizedCallChecker(), + SuspensionPointInsideMutexLockChecker(), JavaClassOnCompanionChecker(), ProtectedInSuperClassCompanionCallChecker(), UnsupportedSyntheticCallableReferenceChecker(), diff --git a/compiler/testData/cli/jvm/suspensionPointInMonitor.out b/compiler/testData/cli/jvm/suspensionPointInMonitor.out index b1eaf67d746..9e41b5fc51d 100644 --- a/compiler/testData/cli/jvm/suspensionPointInMonitor.out +++ b/compiler/testData/cli/jvm/suspensionPointInMonitor.out @@ -1,7 +1,7 @@ -compiler/testData/cli/jvm/suspensionPointInMonitor.kt:26:13: error: the 'suspensionPoint' suspension point is inside a synchronized block +compiler/testData/cli/jvm/suspensionPointInMonitor.kt:26:13: error: the 'suspensionPoint' suspension point is inside a critical section suspensionPoint() ^ -compiler/testData/cli/jvm/suspensionPointInMonitor.kt:60:17: error: the 'suspensionPoint' suspension point is inside a synchronized block +compiler/testData/cli/jvm/suspensionPointInMonitor.kt:60:17: error: the 'suspensionPoint' suspension point is inside a critical section suspensionPoint() ^ COMPILATION_ERROR diff --git a/compiler/testData/diagnostics/testsWithStdLib/coroutines/suspensionPointInMonitor.kt b/compiler/testData/diagnostics/testsWithStdLib/coroutines/suspensionPointInMonitor.kt index 825cea20ddd..350d269d6e0 100644 --- a/compiler/testData/diagnostics/testsWithStdLib/coroutines/suspensionPointInMonitor.kt +++ b/compiler/testData/diagnostics/testsWithStdLib/coroutines/suspensionPointInMonitor.kt @@ -1,7 +1,8 @@ // !DIAGNOSTICS: -UNUSED_PARAMETER, -USELESS_IS_CHECK // SKIP_TXT +import kotlin.concurrent.withLock -val lock = Any() +val lock = java.util.concurrent.locks.ReentrantLock() fun builder(c: suspend () -> Unit) {} @@ -12,14 +13,14 @@ suspend fun suspensionPoint() {} fun test() { builder { synchronized(lock) { - suspensionPoint() + suspensionPoint() } synchronized(lock) label@{ - suspensionPoint() + suspensionPoint() } - synchronized(lock, { suspensionPoint() }) + synchronized(lock, { suspensionPoint() }) synchronized(getLock()) { println("") @@ -30,12 +31,15 @@ fun test() { synchronized(run { getLock() }) { println("") } + lock.withLock { + suspensionPoint() + } } } suspend fun run() { synchronized(lock) { - suspensionPoint() + suspensionPoint() } } @@ -44,7 +48,7 @@ suspend fun ifWhenAndOtherNonsence() { if (lock == Any()) { when (1) { is Int -> { - return@synchronized 1 + returnsInt() + return@synchronized 1 + returnsInt() } else -> {} } diff --git a/compiler/testData/diagnostics/testsWithStdLib/coroutines/suspensionPointInMonitorNewInf.kt b/compiler/testData/diagnostics/testsWithStdLib/coroutines/suspensionPointInMonitorNewInf.kt index bfeda7e1f06..f3ee4b1754a 100644 --- a/compiler/testData/diagnostics/testsWithStdLib/coroutines/suspensionPointInMonitorNewInf.kt +++ b/compiler/testData/diagnostics/testsWithStdLib/coroutines/suspensionPointInMonitorNewInf.kt @@ -2,7 +2,9 @@ // !LANGUAGE: +NewInference // SKIP_TXT -val lock = Any() +import kotlin.concurrent.withLock + +val lock = java.util.concurrent.locks.ReentrantLock() fun builder(c: suspend () -> Unit) {} @@ -13,14 +15,14 @@ suspend fun suspensionPoint() {} fun test() { builder { synchronized(lock) { - suspensionPoint() + suspensionPoint() } synchronized(lock) label@{ - suspensionPoint() + suspensionPoint() } - synchronized(lock, { suspensionPoint() }) + synchronized(lock, { suspensionPoint() }) synchronized(getLock()) { println("") @@ -31,12 +33,15 @@ fun test() { synchronized(run { getLock() }) { println("") } + lock.withLock { + suspensionPoint() + } } } suspend fun run() { synchronized(lock) { - suspensionPoint() + suspensionPoint() } } @@ -45,7 +50,7 @@ suspend fun ifWhenAndOtherNonsence() { if (lock == Any()) { when (1) { is Int -> { - return@synchronized 1 + returnsInt() + return@synchronized 1 + returnsInt() } else -> {} }