azalea/kotlin-fork
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Error on calling suspend functions inside Lock.withLock body

Browse Source 
See SuspensionPointInsideCriticalSectionChecker
This commit is contained in:
Nikita Katkov
2018-12-10 18:15:14 +03:00
committed by Ilmir Usmanov
parent 1d52fb2e26
commit 4db9174b4d
7 changed files with 38 additions and 19 deletions
Download Patch File Download Diff File Expand all files Collapse all files
compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/checkers/SuspensionPointInSynchronizedCallChecker.kt → compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/checkers/SuspensionPointInsideMutexLockChecker.kt
+12 -2
View File
@@ -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.jvm.diagnostics.ErrorsJvm
import org.jetbrains.kotlin.resolve.source.getPsi import org.jetbrains.kotlin.resolve.source.getPsi
class SuspensionPointInSynchronizedCallChecker : CallChecker { class SuspensionPointInsideMutexLockChecker : CallChecker {
override fun check(resolvedCall: ResolvedCall<*>, reportOn: PsiElement, context: CallCheckerContext) { override fun check(resolvedCall: ResolvedCall<*>, reportOn: PsiElement, context: CallCheckerContext) {
val descriptor = resolvedCall.candidateDescriptor val descriptor = resolvedCall.candidateDescriptor
if (descriptor !is FunctionDescriptor || !descriptor.isSuspend) return if (descriptor !is FunctionDescriptor || !descriptor.isSuspend) return
@@ -61,10 +61,20 @@ class SuspensionPointInSynchronizedCallChecker : CallChecker {
if (isSynchronized) { if (isSynchronized) {
val isSecondArgument = (resolved.valueArgumentsByIndex?.get(1) as? ExpressionValueArgument)?.valueArgument == child val isSecondArgument = (resolved.valueArgumentsByIndex?.get(1) as? ExpressionValueArgument)?.valueArgument == child
if (insideLambda && isSecondArgument) { if (insideLambda && isSecondArgument) {
context.trace.report(ErrorsJvm.SUSPENSION_POINT_INSIDE_SYNCHRONIZED.on(reportOn, resolvedCall.resultingDescriptor)) reportProblem(context, reportOn, resolvedCall)
} }
return true return true
} }
val isWithLock = resolved.resultingDescriptor.isTopLevelInPackage("withLock", "kotlin.concurrent")
if (isWithLock) {
reportProblem(context, reportOn, resolvedCall)
return true
}
return false return false
} }
private fun reportProblem(context: CallCheckerContext, reportOn: PsiElement, resolvedCall: ResolvedCall<*>) {
context.trace.report(ErrorsJvm.SUSPENSION_POINT_INSIDE_CRITICAL_SECTION.on(reportOn, resolvedCall.resultingDescriptor))
}
} }
compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/diagnostics/DefaultErrorMessagesJvm.java
+1 -1
View File
@@ -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(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(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_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 = String MESSAGE_FOR_CONCURRENT_HASH_MAP_CONTAINS =
"Method 'contains' from ConcurrentHashMap may have unexpected semantics: it calls 'containsValue' instead of 'containsKey'. " + "Method 'contains' from ConcurrentHashMap may have unexpected semantics: it calls 'containsValue' instead of 'containsKey'. " +
compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/diagnostics/ErrorsJvm.java
+1 -1
View File
@@ -126,7 +126,7 @@ public interface ErrorsJvm {
DiagnosticFactory1<KtAnnotationEntry, String> ANNOTATION_TARGETS_NON_EXISTENT_ACCESSOR = DiagnosticFactory1.create(WARNING); DiagnosticFactory1<KtAnnotationEntry, String> ANNOTATION_TARGETS_NON_EXISTENT_ACCESSOR = DiagnosticFactory1.create(WARNING);
DiagnosticFactory1<PsiElement, String> SUSPENSION_POINT_INSIDE_MONITOR = DiagnosticFactory1.create(ERROR); DiagnosticFactory1<PsiElement, String> SUSPENSION_POINT_INSIDE_MONITOR = DiagnosticFactory1.create(ERROR);
DiagnosticFactory1<PsiElement, CallableDescriptor> SUSPENSION_POINT_INSIDE_SYNCHRONIZED = DiagnosticFactory1.create(ERROR); DiagnosticFactory1<PsiElement, CallableDescriptor> SUSPENSION_POINT_INSIDE_CRITICAL_SECTION = DiagnosticFactory1.create(ERROR);
DiagnosticFactory0<PsiElement> CONCURRENT_HASH_MAP_CONTAINS_OPERATOR = DiagnosticFactory0.create(WARNING); DiagnosticFactory0<PsiElement> CONCURRENT_HASH_MAP_CONTAINS_OPERATOR = DiagnosticFactory0.create(WARNING);
DiagnosticFactory0<PsiElement> CONCURRENT_HASH_MAP_CONTAINS_OPERATOR_ERROR = DiagnosticFactory0.create(ERROR); DiagnosticFactory0<PsiElement> CONCURRENT_HASH_MAP_CONTAINS_OPERATOR_ERROR = DiagnosticFactory0.create(ERROR);
compiler/frontend.java/src/org/jetbrains/kotlin/resolve/jvm/platform/JvmPlatformConfigurator.kt
+1 -1
View File
@@ -42,7 +42,7 @@ object JvmPlatformConfigurator : PlatformConfiguratorBase(
additionalCallCheckers = listOf( additionalCallCheckers = listOf(
JavaAnnotationCallChecker(), JavaAnnotationCallChecker(),
SuspensionPointInSynchronizedCallChecker(), SuspensionPointInsideMutexLockChecker(),
JavaClassOnCompanionChecker(), JavaClassOnCompanionChecker(),
ProtectedInSuperClassCompanionCallChecker(), ProtectedInSuperClassCompanionCallChecker(),
UnsupportedSyntheticCallableReferenceChecker(), UnsupportedSyntheticCallableReferenceChecker(),
compiler/testData/cli/jvm/suspensionPointInMonitor.out
Vendored
+2 -2
View File
@@ -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() 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() suspensionPoint()
^ ^
COMPILATION_ERROR COMPILATION_ERROR
compiler/testData/diagnostics/testsWithStdLib/coroutines/suspensionPointInMonitor.kt
Vendored
+10 -6
View File
@@ -1,7 +1,8 @@
// !DIAGNOSTICS: -UNUSED_PARAMETER, -USELESS_IS_CHECK // !DIAGNOSTICS: -UNUSED_PARAMETER, -USELESS_IS_CHECK
// SKIP_TXT // SKIP_TXT
import kotlin.concurrent.withLock
val lock = Any() val lock = java.util.concurrent.locks.ReentrantLock()
fun builder(c: suspend () -> Unit) {} fun builder(c: suspend () -> Unit) {}
@@ -12,14 +13,14 @@ suspend fun suspensionPoint() {}
fun test() { fun test() {
builder { builder {
synchronized(lock) { synchronized(lock) {
<!SUSPENSION_POINT_INSIDE_SYNCHRONIZED!>suspensionPoint<!>() <!SUSPENSION_POINT_INSIDE_CRITICAL_SECTION!>suspensionPoint<!>()
} }
synchronized(lock) label@{ synchronized(lock) label@{
<!SUSPENSION_POINT_INSIDE_SYNCHRONIZED!>suspensionPoint<!>() <!SUSPENSION_POINT_INSIDE_CRITICAL_SECTION!>suspensionPoint<!>()
} }
synchronized(lock, { <!SUSPENSION_POINT_INSIDE_SYNCHRONIZED!>suspensionPoint<!>() }) synchronized(lock, { <!SUSPENSION_POINT_INSIDE_CRITICAL_SECTION!>suspensionPoint<!>() })
synchronized(getLock()) { synchronized(getLock()) {
println("") println("")
@@ -30,12 +31,15 @@ fun test() {
synchronized(run { getLock() }) { synchronized(run { getLock() }) {
println("") println("")
} }
lock.withLock {
<!SUSPENSION_POINT_INSIDE_CRITICAL_SECTION!>suspensionPoint<!>()
}
} }
} }
suspend fun run() { suspend fun run() {
synchronized(lock) { synchronized(lock) {
<!SUSPENSION_POINT_INSIDE_SYNCHRONIZED!>suspensionPoint<!>() <!SUSPENSION_POINT_INSIDE_CRITICAL_SECTION!>suspensionPoint<!>()
} }
} }
@@ -44,7 +48,7 @@ suspend fun ifWhenAndOtherNonsence() {
if (lock == Any()) { if (lock == Any()) {
when (1) { when (1) {
is Int -> { is Int -> {
return@synchronized 1 + <!SUSPENSION_POINT_INSIDE_SYNCHRONIZED!>returnsInt<!>() return@synchronized 1 + <!SUSPENSION_POINT_INSIDE_CRITICAL_SECTION!>returnsInt<!>()
} }
else -> {} else -> {}
} }
compiler/testData/diagnostics/testsWithStdLib/coroutines/suspensionPointInMonitorNewInf.kt
Vendored
+11 -6
View File
@@ -2,7 +2,9 @@
// !LANGUAGE: +NewInference // !LANGUAGE: +NewInference
// SKIP_TXT // SKIP_TXT
val lock = Any() import kotlin.concurrent.withLock
val lock = java.util.concurrent.locks.ReentrantLock()
fun builder(c: suspend () -> Unit) {} fun builder(c: suspend () -> Unit) {}
@@ -13,14 +15,14 @@ suspend fun suspensionPoint() {}
fun test() { fun test() {
builder { builder {
synchronized(lock) { synchronized(lock) {
<!SUSPENSION_POINT_INSIDE_SYNCHRONIZED!>suspensionPoint<!>() <!SUSPENSION_POINT_INSIDE_CRITICAL_SECTION!>suspensionPoint<!>()
} }
synchronized(lock) label@{ synchronized(lock) label@{
<!SUSPENSION_POINT_INSIDE_SYNCHRONIZED!>suspensionPoint<!>() <!SUSPENSION_POINT_INSIDE_CRITICAL_SECTION!>suspensionPoint<!>()
} }
synchronized(lock, { <!SUSPENSION_POINT_INSIDE_SYNCHRONIZED!>suspensionPoint<!>() }) synchronized(lock, { <!SUSPENSION_POINT_INSIDE_CRITICAL_SECTION!>suspensionPoint<!>() })
synchronized(getLock()) { synchronized(getLock()) {
println("") println("")
@@ -31,12 +33,15 @@ fun test() {
synchronized(run { getLock() }) { synchronized(run { getLock() }) {
println("") println("")
} }
lock.withLock {
<!SUSPENSION_POINT_INSIDE_CRITICAL_SECTION!>suspensionPoint<!>()
}
} }
} }
suspend fun run() { suspend fun run() {
synchronized(lock) { synchronized(lock) {
<!SUSPENSION_POINT_INSIDE_SYNCHRONIZED!>suspensionPoint<!>() <!SUSPENSION_POINT_INSIDE_CRITICAL_SECTION!>suspensionPoint<!>()
} }
} }
@@ -45,7 +50,7 @@ suspend fun ifWhenAndOtherNonsence() {
if (lock == Any()) { if (lock == Any()) {
when (1) { when (1) {
is Int -> { is Int -> {
return@synchronized 1 + <!SUSPENSION_POINT_INSIDE_SYNCHRONIZED!>returnsInt<!>() return@synchronized 1 + <!SUSPENSION_POINT_INSIDE_CRITICAL_SECTION!>returnsInt<!>()
} }
else -> {} else -> {}
} }