From 44356f7105499939000f6dfcf1427baa943789e8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?No=C3=A9mie=20B=C3=A9nard?= Date: Wed, 5 Aug 2026 14:20:39 +0200 Subject: [PATCH 1/8] Add rule metadata --- .../org/sonar/l10n/java/rules/java/S9141.html | 86 +++++++++++++++++++ .../org/sonar/l10n/java/rules/java/S9141.json | 26 ++++++ .../main/resources/profiles/Sonar_way/S9141 | 0 3 files changed, 112 insertions(+) create mode 100644 sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9141.html create mode 100644 sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9141.json create mode 100644 sonar-java-plugin/src/main/resources/profiles/Sonar_way/S9141 diff --git a/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9141.html b/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9141.html new file mode 100644 index 00000000000..4388cb5f037 --- /dev/null +++ b/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9141.html @@ -0,0 +1,86 @@ +

This is an issue when a language-level synchronization primitive is used on an object from a standard concurrency library, such as reentrant locks, +semaphores, countdown latches, cyclic barriers, or thread-safe queue implementations.

+

In Java, this specifically refers to using synchronized blocks on objects from the java.util.concurrent package, such as +ReentrantLock, Semaphore, CountDownLatch, CyclicBarrier, or BlockingQueue +implementations.

+

Why is this an issue?

+

Concurrent programming libraries provide their own high-level synchronization mechanisms. These are designed to be more flexible and powerful than +the language’s basic object locking mechanism.

+

When you use the basic locking mechanism on such an object, you are acquiring a lock that is built into the object itself at the language runtime +level. However, this built-in lock is completely separate from the object’s own synchronization protocol. The two mechanisms do not interact with each +other at all.

+

For example:

+ +

This means that threads using the basic locking mechanism on these objects will not coordinate with threads using the object’s proper API methods. +Different threads might think they have exclusive access when they actually don’t, leading to race conditions and data corruption.

+

This pattern typically indicates a misunderstanding of how concurrent library classes work. The developer likely intended to use the object’s own +synchronization mechanism but mistakenly used the basic locking mechanism instead.

+

In Java, this refers to classes from the java.util.concurrent package and using the synchronized keyword. Specific +examples include using synchronized(reentrantLock) instead of calling lock()/unlock() on a +ReentrantLock, or using synchronized(semaphore) instead of calling acquire()/release() on a +Semaphore.

+

What is the potential impact?

+

When language-level locking mechanisms are used on concurrent data structures that implement their own internal synchronization, the code fails to +provide the expected thread-safety guarantees:

+ +

These issues are particularly dangerous because they often only manifest under specific timing conditions, making them hard to detect during +testing.

+

In Java, this specifically refers to using synchronized blocks or methods on java.util.concurrent synchronization objects +like ReentrantLock, Semaphore, CountDownLatch, CyclicBarrier, or BlockingQueue +implementations.

+

How to fix it

+

Replace the synchronized block with the correct API methods for the specific java.util.concurrent class you’re using. +Always use try-finally blocks when acquiring locks to ensure they are released even if an exception occurs.

+

Code examples

+

Noncompliant code example

+
+private final ReentrantLock lock = new ReentrantLock();
+
+public void doWork() {
+    synchronized (lock) {  // Noncompliant
+        criticalSection();
+    }
+}
+
+

Compliant solution

+
+private final ReentrantLock lock = new ReentrantLock();
+
+public void doWork() {
+    lock.lock();
+    try {
+        criticalSection();
+    } finally {
+        lock.unlock();
+    }
+}
+
+

Resources

+

Documentation

+ +

Related rules

+ + diff --git a/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9141.json b/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9141.json new file mode 100644 index 00000000000..c76a01f735d --- /dev/null +++ b/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9141.json @@ -0,0 +1,26 @@ +{ + "title": "Intrinsic locks should not be used on \"java.util.concurrent\" objects", + "type": "BUG", + "status": "ready", + "remediation": { + "func": "Constant\/Issue", + "constantCost": "10 min" + }, + "tags": [ + "concurrency", + "multi-threading", + "pitfall" + ], + "defaultSeverity": "Critical", + "ruleSpecification": "RSPEC-9141", + "sqKey": "S9141", + "scope": "All", + "quickfix": "unknown", + "code": { + "impacts": { + "RELIABILITY": "HIGH", + "MAINTAINABILITY": "MEDIUM" + }, + "attribute": "LOGICAL" + } +} diff --git a/sonar-java-plugin/src/main/resources/profiles/Sonar_way/S9141 b/sonar-java-plugin/src/main/resources/profiles/Sonar_way/S9141 new file mode 100644 index 00000000000..e69de29bb2d From de5f628714f1cb4c916fe7c4c84d5bcd0b8f17db Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?No=C3=A9mie=20B=C3=A9nard?= Date: Wed, 5 Aug 2026 14:30:29 +0200 Subject: [PATCH 2/8] Add failing reproducer --- ...hronizedOnConcurrentObjectCheckSample.java | 84 +++++++++++++++++++ ...nchronizedOnConcurrentObjectCheckTest.java | 34 ++++++++ 2 files changed, 118 insertions(+) create mode 100644 java-checks-test-sources/default/src/main/java/checks/SynchronizedOnConcurrentObjectCheckSample.java create mode 100644 java-checks/src/test/java/org/sonar/java/checks/SynchronizedOnConcurrentObjectCheckTest.java diff --git a/java-checks-test-sources/default/src/main/java/checks/SynchronizedOnConcurrentObjectCheckSample.java b/java-checks-test-sources/default/src/main/java/checks/SynchronizedOnConcurrentObjectCheckSample.java new file mode 100644 index 00000000000..0e35eb0c3bc --- /dev/null +++ b/java-checks-test-sources/default/src/main/java/checks/SynchronizedOnConcurrentObjectCheckSample.java @@ -0,0 +1,84 @@ +package checks; + +import java.util.concurrent.ArrayBlockingQueue; +import java.util.concurrent.BlockingQueue; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.CyclicBarrier; +import java.util.concurrent.LinkedBlockingQueue; +import java.util.concurrent.Semaphore; +import java.util.concurrent.locks.Lock; +import java.util.concurrent.locks.ReentrantLock; +import java.util.concurrent.locks.ReentrantReadWriteLock; + +class SynchronizedOnConcurrentObjectCheckSample { + + private final ReentrantLock reentrantLock = new ReentrantLock(); + private final ReentrantReadWriteLock rwLock = new ReentrantReadWriteLock(); + private final Lock lock = new ReentrantLock(); + private final Semaphore semaphore = new Semaphore(1); + private final CountDownLatch latch = new CountDownLatch(1); + private final CyclicBarrier barrier = new CyclicBarrier(2); + private final BlockingQueue blockingQueue = new ArrayBlockingQueue<>(10); + private final ArrayBlockingQueue arrayBlockingQueue = new ArrayBlockingQueue<>(10); + private final LinkedBlockingQueue linkedBlockingQueue = new LinkedBlockingQueue<>(); + + private final Object objectLock = new Object(); + + void noncompliant() { + synchronized (reentrantLock) { // Noncompliant {{Use the "ReentrantLock" API for synchronization instead of a "synchronized" block.}} + // ^^^^^^^^^^^^^ + } + + synchronized (lock) { // Noncompliant {{Use the "ReentrantLock" API for synchronization instead of a "synchronized" block.}} + // ^^^^ + } + + synchronized (rwLock) { // Noncompliant {{Use the "ReentrantReadWriteLock" API for synchronization instead of a "synchronized" block.}} + // ^^^^^^ + } + + synchronized (semaphore) { // Noncompliant {{Use the "Semaphore" API for synchronization instead of a "synchronized" block.}} + // ^^^^^^^^^ + } + + synchronized (latch) { // Noncompliant {{Use the "CountDownLatch" API for synchronization instead of a "synchronized" block.}} + // ^^^^^ + } + + synchronized (barrier) { // Noncompliant {{Use the "CyclicBarrier" API for synchronization instead of a "synchronized" block.}} + // ^^^^^^^ + } + + synchronized (blockingQueue) { // Noncompliant {{Use the "ArrayBlockingQueue" API for synchronization instead of a "synchronized" block.}} + // ^^^^^^^^^^^^ + } + + synchronized (arrayBlockingQueue) { // Noncompliant {{Use the "ArrayBlockingQueue" API for synchronization instead of a "synchronized" block.}} + // ^^^^^^^^^^^^^^^^^^ + } + + synchronized (linkedBlockingQueue) { // Noncompliant {{Use the "LinkedBlockingQueue" API for synchronization instead of a "synchronized" block.}} + // ^^^^^^^^^^^^^^^^^^^ + } + } + + void compliant() { + synchronized (objectLock) { + // ... + } + + reentrantLock.lock(); + try { + // ... + } finally { + reentrantLock.unlock(); + } + + rwLock.writeLock().lock(); + try { + // ... + } finally { + rwLock.writeLock().unlock(); + } + } +} diff --git a/java-checks/src/test/java/org/sonar/java/checks/SynchronizedOnConcurrentObjectCheckTest.java b/java-checks/src/test/java/org/sonar/java/checks/SynchronizedOnConcurrentObjectCheckTest.java new file mode 100644 index 00000000000..d12fcd4ea9b --- /dev/null +++ b/java-checks/src/test/java/org/sonar/java/checks/SynchronizedOnConcurrentObjectCheckTest.java @@ -0,0 +1,34 @@ +/* + * SonarQube Java + * Copyright (C) SonarSource Sàrl + * mailto:info AT sonarsource DOT com + * + * You can redistribute and/or modify this program under the terms of + * the Sonar Source-Available License Version 1, as published by SonarSource Sàrl. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. + * See the Sonar Source-Available License for more details. + * + * You should have received a copy of the Sonar Source-Available License + * along with this program; if not, see https://sonarsource.com/license/ssal/ + */ +package org.sonar.java.checks; + +import org.junit.jupiter.api.Test; +import org.sonar.java.checks.verifier.CheckVerifier; + +import static org.sonar.java.checks.verifier.TestUtils.mainCodeSourcesPath; + +class SynchronizedOnConcurrentObjectCheckTest { + + @Test + void test() { + CheckVerifier.newVerifier() + .onFile(mainCodeSourcesPath("checks/SynchronizedOnConcurrentObjectCheckSample.java")) + .withCheck(new SynchronizedOnConcurrentObjectCheck()) + .verifyIssues(); + } + +} From 85b7a23421e2b5970afbcd09d47b6bf422f94d3e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?No=C3=A9mie=20B=C3=A9nard?= Date: Wed, 5 Aug 2026 14:48:04 +0200 Subject: [PATCH 3/8] Implement rule --- ...hronizedOnConcurrentObjectCheckSample.java | 26 ++++++++- .../SynchronizedOnConcurrentObjectCheck.java | 56 +++++++++++++++++++ 2 files changed, 79 insertions(+), 3 deletions(-) create mode 100644 java-checks/src/main/java/org/sonar/java/checks/SynchronizedOnConcurrentObjectCheck.java diff --git a/java-checks-test-sources/default/src/main/java/checks/SynchronizedOnConcurrentObjectCheckSample.java b/java-checks-test-sources/default/src/main/java/checks/SynchronizedOnConcurrentObjectCheckSample.java index 0e35eb0c3bc..016bc1ee9e4 100644 --- a/java-checks-test-sources/default/src/main/java/checks/SynchronizedOnConcurrentObjectCheckSample.java +++ b/java-checks-test-sources/default/src/main/java/checks/SynchronizedOnConcurrentObjectCheckSample.java @@ -6,6 +6,8 @@ import java.util.concurrent.CyclicBarrier; import java.util.concurrent.LinkedBlockingQueue; import java.util.concurrent.Semaphore; +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.locks.Lock; import java.util.concurrent.locks.ReentrantLock; import java.util.concurrent.locks.ReentrantReadWriteLock; @@ -21,6 +23,9 @@ class SynchronizedOnConcurrentObjectCheckSample { private final BlockingQueue blockingQueue = new ArrayBlockingQueue<>(10); private final ArrayBlockingQueue arrayBlockingQueue = new ArrayBlockingQueue<>(10); private final LinkedBlockingQueue linkedBlockingQueue = new LinkedBlockingQueue<>(); + private final AtomicBoolean atomicBoolean = new AtomicBoolean(); + private final AtomicInteger atomicInteger = new AtomicInteger(); + private final CustomLock customLock = new CustomLock(); private final Object objectLock = new Object(); @@ -29,7 +34,7 @@ void noncompliant() { // ^^^^^^^^^^^^^ } - synchronized (lock) { // Noncompliant {{Use the "ReentrantLock" API for synchronization instead of a "synchronized" block.}} + synchronized (lock) { // Noncompliant {{Use the "Lock" API for synchronization instead of a "synchronized" block.}} // ^^^^ } @@ -49,8 +54,8 @@ void noncompliant() { // ^^^^^^^ } - synchronized (blockingQueue) { // Noncompliant {{Use the "ArrayBlockingQueue" API for synchronization instead of a "synchronized" block.}} - // ^^^^^^^^^^^^ + synchronized (blockingQueue) { // Noncompliant {{Use the "BlockingQueue" API for synchronization instead of a "synchronized" block.}} + // ^^^^^^^^^^^^^ } synchronized (arrayBlockingQueue) { // Noncompliant {{Use the "ArrayBlockingQueue" API for synchronization instead of a "synchronized" block.}} @@ -60,6 +65,18 @@ void noncompliant() { synchronized (linkedBlockingQueue) { // Noncompliant {{Use the "LinkedBlockingQueue" API for synchronization instead of a "synchronized" block.}} // ^^^^^^^^^^^^^^^^^^^ } + + synchronized (atomicBoolean) { // Noncompliant {{Use the "AtomicBoolean" API for synchronization instead of a "synchronized" block.}} + // ^^^^^^^^^^^^^ + } + + synchronized (atomicInteger) { // Noncompliant {{Use the "AtomicInteger" API for synchronization instead of a "synchronized" block.}} + // ^^^^^^^^^^^^^ + } + + synchronized (customLock) { // Noncompliant {{Use the "CustomLock" API for synchronization instead of a "synchronized" block.}} + // ^^^^^^^^^^ + } } void compliant() { @@ -81,4 +98,7 @@ void compliant() { rwLock.writeLock().unlock(); } } + + static class CustomLock extends ReentrantLock { + } } diff --git a/java-checks/src/main/java/org/sonar/java/checks/SynchronizedOnConcurrentObjectCheck.java b/java-checks/src/main/java/org/sonar/java/checks/SynchronizedOnConcurrentObjectCheck.java new file mode 100644 index 00000000000..793fe6b8435 --- /dev/null +++ b/java-checks/src/main/java/org/sonar/java/checks/SynchronizedOnConcurrentObjectCheck.java @@ -0,0 +1,56 @@ +/* + * SonarQube Java + * Copyright (C) SonarSource Sàrl + * mailto:info AT sonarsource DOT com + * + * You can redistribute and/or modify this program under the terms of + * the Sonar Source-Available License Version 1, as published by SonarSource Sàrl. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. + * See the Sonar Source-Available License for more details. + * + * You should have received a copy of the Sonar Source-Available License + * along with this program; if not, see https://sonarsource.com/license/ssal/ + */ +package org.sonar.java.checks; + +import java.util.Collections; +import java.util.List; +import org.sonar.check.Rule; +import org.sonar.plugins.java.api.IssuableSubscriptionVisitor; +import org.sonar.plugins.java.api.semantic.Type; +import org.sonar.plugins.java.api.tree.ExpressionTree; +import org.sonar.plugins.java.api.tree.SynchronizedStatementTree; +import org.sonar.plugins.java.api.tree.Tree; + +@Rule(key = "S9141") +public class SynchronizedOnConcurrentObjectCheck extends IssuableSubscriptionVisitor { + + private static final String CONCURRENT_PACKAGE_PREFIX = "java.util.concurrent."; + + @Override + public List nodesToVisit() { + return Collections.singletonList(Tree.Kind.SYNCHRONIZED_STATEMENT); + } + + @Override + public void visitNode(Tree tree) { + ExpressionTree expression = ((SynchronizedStatementTree) tree).expression(); + Type expressionType = expression.symbolType(); + if (isFromConcurrentPackage(expressionType)) { + reportIssue(expression, String.format( + "Use the \"%s\" API for synchronization instead of a \"synchronized\" block.", expressionType.name())); + } + } + + private static boolean isFromConcurrentPackage(Type type) { + if (type.fullyQualifiedName().startsWith(CONCURRENT_PACKAGE_PREFIX)) { + return true; + } + return type.symbol().superTypes().stream() + .anyMatch(superType -> superType.fullyQualifiedName().startsWith(CONCURRENT_PACKAGE_PREFIX)); + } + +} From 81bde3f5998b964c35e6c507c3f2cf317a326369 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?No=C3=A9mie=20B=C3=A9nard?= Date: Wed, 5 Aug 2026 16:04:15 +0200 Subject: [PATCH 4/8] Update ruling expectations --- .../src/test/resources/autoscan/diffs/diff_S9141.json | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 its/autoscan/src/test/resources/autoscan/diffs/diff_S9141.json diff --git a/its/autoscan/src/test/resources/autoscan/diffs/diff_S9141.json b/its/autoscan/src/test/resources/autoscan/diffs/diff_S9141.json new file mode 100644 index 00000000000..4181b1eb2a5 --- /dev/null +++ b/its/autoscan/src/test/resources/autoscan/diffs/diff_S9141.json @@ -0,0 +1,6 @@ +{ + "ruleKey": "S9141", + "hasTruePositives": true, + "falseNegatives": 0, + "falsePositives": 0 +} From 42a1522ac4a132ecc87a95ed35285c01fca3cd97 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?No=C3=A9mie=20B=C3=A9nard?= Date: Wed, 5 Aug 2026 16:31:00 +0200 Subject: [PATCH 5/8] Fix review comments --- ...hronizedOnConcurrentObjectCheckSample.java | 12 ++++++++ .../SynchronizedOnConcurrentObjectCheck.java | 30 ++++++++++++++----- 2 files changed, 34 insertions(+), 8 deletions(-) diff --git a/java-checks-test-sources/default/src/main/java/checks/SynchronizedOnConcurrentObjectCheckSample.java b/java-checks-test-sources/default/src/main/java/checks/SynchronizedOnConcurrentObjectCheckSample.java index 016bc1ee9e4..21b1aaf42a8 100644 --- a/java-checks-test-sources/default/src/main/java/checks/SynchronizedOnConcurrentObjectCheckSample.java +++ b/java-checks-test-sources/default/src/main/java/checks/SynchronizedOnConcurrentObjectCheckSample.java @@ -2,8 +2,10 @@ import java.util.concurrent.ArrayBlockingQueue; import java.util.concurrent.BlockingQueue; +import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.CountDownLatch; import java.util.concurrent.CyclicBarrier; +import java.util.concurrent.Future; import java.util.concurrent.LinkedBlockingQueue; import java.util.concurrent.Semaphore; import java.util.concurrent.atomic.AtomicBoolean; @@ -28,6 +30,8 @@ class SynchronizedOnConcurrentObjectCheckSample { private final CustomLock customLock = new CustomLock(); private final Object objectLock = new Object(); + private final ConcurrentHashMap concurrentMap = new ConcurrentHashMap<>(); + private Future future; void noncompliant() { synchronized (reentrantLock) { // Noncompliant {{Use the "ReentrantLock" API for synchronization instead of a "synchronized" block.}} @@ -84,6 +88,14 @@ void compliant() { // ... } + synchronized (concurrentMap) { + // ... + } + + synchronized (future) { + // ... + } + reentrantLock.lock(); try { // ... diff --git a/java-checks/src/main/java/org/sonar/java/checks/SynchronizedOnConcurrentObjectCheck.java b/java-checks/src/main/java/org/sonar/java/checks/SynchronizedOnConcurrentObjectCheck.java index 793fe6b8435..d537b848621 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/SynchronizedOnConcurrentObjectCheck.java +++ b/java-checks/src/main/java/org/sonar/java/checks/SynchronizedOnConcurrentObjectCheck.java @@ -18,6 +18,7 @@ import java.util.Collections; import java.util.List; +import java.util.Set; import org.sonar.check.Rule; import org.sonar.plugins.java.api.IssuableSubscriptionVisitor; import org.sonar.plugins.java.api.semantic.Type; @@ -28,7 +29,17 @@ @Rule(key = "S9141") public class SynchronizedOnConcurrentObjectCheck extends IssuableSubscriptionVisitor { - private static final String CONCURRENT_PACKAGE_PREFIX = "java.util.concurrent."; + private static final String CONCURRENT_LOCKS_PREFIX = "java.util.concurrent.locks."; + private static final String CONCURRENT_ATOMIC_PREFIX = "java.util.concurrent.atomic."; + private static final Set CONCURRENT_SYNC_TYPES = Set.of( + "java.util.concurrent.Semaphore", + "java.util.concurrent.CountDownLatch", + "java.util.concurrent.CyclicBarrier", + "java.util.concurrent.Exchanger", + "java.util.concurrent.Phaser", + "java.util.concurrent.BlockingQueue", + "java.util.concurrent.BlockingDeque", + "java.util.concurrent.TransferQueue"); @Override public List nodesToVisit() { @@ -39,18 +50,21 @@ public List nodesToVisit() { public void visitNode(Tree tree) { ExpressionTree expression = ((SynchronizedStatementTree) tree).expression(); Type expressionType = expression.symbolType(); - if (isFromConcurrentPackage(expressionType)) { + if (isConcurrentSyncPrimitive(expressionType)) { reportIssue(expression, String.format( "Use the \"%s\" API for synchronization instead of a \"synchronized\" block.", expressionType.name())); } } - private static boolean isFromConcurrentPackage(Type type) { - if (type.fullyQualifiedName().startsWith(CONCURRENT_PACKAGE_PREFIX)) { - return true; - } - return type.symbol().superTypes().stream() - .anyMatch(superType -> superType.fullyQualifiedName().startsWith(CONCURRENT_PACKAGE_PREFIX)); + private static boolean isConcurrentSyncPrimitive(Type type) { + return isKnownSyncPrimitive(type) || type.symbol().superTypes().stream().anyMatch(SynchronizedOnConcurrentObjectCheck::isKnownSyncPrimitive); + } + + private static boolean isKnownSyncPrimitive(Type type) { + String fqn = type.fullyQualifiedName(); + return fqn.startsWith(CONCURRENT_LOCKS_PREFIX) + || fqn.startsWith(CONCURRENT_ATOMIC_PREFIX) + || CONCURRENT_SYNC_TYPES.contains(fqn); } } From d45782936b351b10ed0d53381c3e10b268586284 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?No=C3=A9mie=20B=C3=A9nard?= Date: Wed, 5 Aug 2026 16:36:36 +0200 Subject: [PATCH 6/8] Update rule metadata --- .../org/sonar/l10n/java/rules/java/S9141.html | 18 ++++-------------- 1 file changed, 4 insertions(+), 14 deletions(-) diff --git a/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9141.html b/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9141.html index 4388cb5f037..b28d782336e 100644 --- a/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9141.html +++ b/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9141.html @@ -1,6 +1,4 @@ -

This is an issue when a language-level synchronization primitive is used on an object from a standard concurrency library, such as reentrant locks, -semaphores, countdown latches, cyclic barriers, or thread-safe queue implementations.

-

In Java, this specifically refers to using synchronized blocks on objects from the java.util.concurrent package, such as +

This is an issue when using synchronized blocks on objects from the java.util.concurrent package, such as ReentrantLock, Semaphore, CountDownLatch, CyclicBarrier, or BlockingQueue implementations.

Why is this an issue?

@@ -36,9 +34,6 @@

What is the potential impact?

These issues are particularly dangerous because they often only manifest under specific timing conditions, making them hard to detect during testing.

-

In Java, this specifically refers to using synchronized blocks or methods on java.util.concurrent synchronization objects -like ReentrantLock, Semaphore, CountDownLatch, CyclicBarrier, or BlockingQueue -implementations.

How to fix it

Replace the synchronized block with the correct API methods for the specific java.util.concurrent class you’re using. Always use try-finally blocks when acquiring locks to ensure they are released even if an exception occurs.

@@ -70,14 +65,9 @@

Resources

Documentation

Related rules

    From 2cb59a42d74a4e00cca779cc26074bfcadb9f891 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?No=C3=A9mie=20B=C3=A9nard?= Date: Thu, 6 Aug 2026 11:12:16 +0200 Subject: [PATCH 7/8] Add test without semantic --- .../checks/SynchronizedOnConcurrentObjectCheckTest.java | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/java-checks/src/test/java/org/sonar/java/checks/SynchronizedOnConcurrentObjectCheckTest.java b/java-checks/src/test/java/org/sonar/java/checks/SynchronizedOnConcurrentObjectCheckTest.java index d12fcd4ea9b..72ac10b1a2b 100644 --- a/java-checks/src/test/java/org/sonar/java/checks/SynchronizedOnConcurrentObjectCheckTest.java +++ b/java-checks/src/test/java/org/sonar/java/checks/SynchronizedOnConcurrentObjectCheckTest.java @@ -31,4 +31,13 @@ void test() { .verifyIssues(); } + @Test + void test_without_semantic() { + CheckVerifier.newVerifier() + .onFile(mainCodeSourcesPath("checks/SynchronizedOnConcurrentObjectCheckSample.java")) + .withCheck(new SynchronizedOnConcurrentObjectCheck()) + .withoutSemantic() + .verifyIssues(); + } + } From f78f122245a2101d449b664eb43a74ec5147bf08 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?No=C3=A9mie=20B=C3=A9nard?= Date: Thu, 6 Aug 2026 17:07:41 +0200 Subject: [PATCH 8/8] Merge S9141 into S2442 --- .../resources/autoscan/diffs/diff_S9141.json | 6 -- .../checks/SynchronizedLockCheckSample.java | 42 ---------- ...hronizedOnConcurrentObjectCheckSample.java | 28 +++++-- .../java/checks/SynchronizedLockCheck.java | 45 ----------- .../SynchronizedOnConcurrentObjectCheck.java | 19 +++-- .../checks/SynchronizedLockCheckTest.java | 33 -------- .../org/sonar/l10n/java/rules/java/S9141.html | 76 ------------------- .../org/sonar/l10n/java/rules/java/S9141.json | 26 ------- .../main/resources/profiles/Sonar_way/S9141 | 0 9 files changed, 34 insertions(+), 241 deletions(-) delete mode 100644 its/autoscan/src/test/resources/autoscan/diffs/diff_S9141.json delete mode 100644 java-checks-test-sources/default/src/main/java/checks/SynchronizedLockCheckSample.java delete mode 100644 java-checks/src/main/java/org/sonar/java/checks/SynchronizedLockCheck.java delete mode 100644 java-checks/src/test/java/org/sonar/java/checks/SynchronizedLockCheckTest.java delete mode 100644 sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9141.html delete mode 100644 sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9141.json delete mode 100644 sonar-java-plugin/src/main/resources/profiles/Sonar_way/S9141 diff --git a/its/autoscan/src/test/resources/autoscan/diffs/diff_S9141.json b/its/autoscan/src/test/resources/autoscan/diffs/diff_S9141.json deleted file mode 100644 index 4181b1eb2a5..00000000000 --- a/its/autoscan/src/test/resources/autoscan/diffs/diff_S9141.json +++ /dev/null @@ -1,6 +0,0 @@ -{ - "ruleKey": "S9141", - "hasTruePositives": true, - "falseNegatives": 0, - "falsePositives": 0 -} diff --git a/java-checks-test-sources/default/src/main/java/checks/SynchronizedLockCheckSample.java b/java-checks-test-sources/default/src/main/java/checks/SynchronizedLockCheckSample.java deleted file mode 100644 index ecb8eb4112f..00000000000 --- a/java-checks-test-sources/default/src/main/java/checks/SynchronizedLockCheckSample.java +++ /dev/null @@ -1,42 +0,0 @@ -package checks; - -import java.util.concurrent.TimeUnit; -import java.util.concurrent.locks.Condition; -import java.util.concurrent.locks.Lock; - -class SynchronizedLockCheckSample { - void foo() { - Lock lock = new MyLockImpl(); - synchronized (lock) { // Noncompliant {{Synchronize on this "Lock" object using "acquire/release".}} -// ^^^^ - } - synchronized (new MyLockImpl()) { // Noncompliant {{Synchronize on this "Lock" object using "acquire/release".}} - } - synchronized (new UselessIncrementCheck()) { // Compliant - } - } -} - -class MyLockImpl implements Lock { - @Override - public void lock() { - } - @Override - public void lockInterruptibly() throws InterruptedException { - } - @Override - public boolean tryLock() { - return false; - } - @Override - public boolean tryLock(long time, TimeUnit unit) throws InterruptedException { - return false; - } - @Override - public void unlock() { - } - @Override - public Condition newCondition() { - return null; - } -} diff --git a/java-checks-test-sources/default/src/main/java/checks/SynchronizedOnConcurrentObjectCheckSample.java b/java-checks-test-sources/default/src/main/java/checks/SynchronizedOnConcurrentObjectCheckSample.java index 21b1aaf42a8..05233a7a1e3 100644 --- a/java-checks-test-sources/default/src/main/java/checks/SynchronizedOnConcurrentObjectCheckSample.java +++ b/java-checks-test-sources/default/src/main/java/checks/SynchronizedOnConcurrentObjectCheckSample.java @@ -8,8 +8,10 @@ import java.util.concurrent.Future; import java.util.concurrent.LinkedBlockingQueue; import java.util.concurrent.Semaphore; +import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicInteger; +import java.util.concurrent.locks.Condition; import java.util.concurrent.locks.Lock; import java.util.concurrent.locks.ReentrantLock; import java.util.concurrent.locks.ReentrantReadWriteLock; @@ -28,6 +30,7 @@ class SynchronizedOnConcurrentObjectCheckSample { private final AtomicBoolean atomicBoolean = new AtomicBoolean(); private final AtomicInteger atomicInteger = new AtomicInteger(); private final CustomLock customLock = new CustomLock(); + private final CustomLockImpl customLockImpl = new CustomLockImpl(); private final Object objectLock = new Object(); private final ConcurrentHashMap concurrentMap = new ConcurrentHashMap<>(); @@ -81,36 +84,51 @@ void noncompliant() { synchronized (customLock) { // Noncompliant {{Use the "CustomLock" API for synchronization instead of a "synchronized" block.}} // ^^^^^^^^^^ } + + synchronized (customLockImpl) { // Noncompliant {{Use the "CustomLockImpl" API for synchronization instead of a "synchronized" block.}} + // ^^^^^^^^^^^^^^ + } } void compliant() { synchronized (objectLock) { - // ... } synchronized (concurrentMap) { - // ... } synchronized (future) { - // ... } reentrantLock.lock(); try { - // ... } finally { reentrantLock.unlock(); } rwLock.writeLock().lock(); try { - // ... } finally { rwLock.writeLock().unlock(); } } + void example() { + var lock2 = new ReentrantLock(); + synchronized (lock2) { // Noncompliant + } + } + static class CustomLock extends ReentrantLock { } + + // Custom Lock implementation outside java.util.concurrent.locks — caught via isSubtypeOf(Lock) + static class CustomLockImpl implements Lock { + @Override public void lock() {} + @Override public void lockInterruptibly() throws InterruptedException {} + @Override public boolean tryLock() { return false; } + @Override public boolean tryLock(long time, TimeUnit unit) throws InterruptedException { return false; } + @Override public void unlock() {} + @Override public Condition newCondition() { return null; } + } } diff --git a/java-checks/src/main/java/org/sonar/java/checks/SynchronizedLockCheck.java b/java-checks/src/main/java/org/sonar/java/checks/SynchronizedLockCheck.java deleted file mode 100644 index 23e523142b3..00000000000 --- a/java-checks/src/main/java/org/sonar/java/checks/SynchronizedLockCheck.java +++ /dev/null @@ -1,45 +0,0 @@ -/* - * SonarQube Java - * Copyright (C) SonarSource Sàrl - * mailto:info AT sonarsource DOT com - * - * You can redistribute and/or modify this program under the terms of - * the Sonar Source-Available License Version 1, as published by SonarSource Sàrl. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. - * See the Sonar Source-Available License for more details. - * - * You should have received a copy of the Sonar Source-Available License - * along with this program; if not, see https://sonarsource.com/license/ssal/ - */ -package org.sonar.java.checks; - -import org.sonar.check.Rule; -import org.sonar.plugins.java.api.IssuableSubscriptionVisitor; -import org.sonar.plugins.java.api.tree.ExpressionTree; -import org.sonar.plugins.java.api.tree.SynchronizedStatementTree; -import org.sonar.plugins.java.api.tree.Tree; -import org.sonar.plugins.java.api.tree.Tree.Kind; - -import java.util.Collections; -import java.util.List; - -@Rule(key = "S2442") -public class SynchronizedLockCheck extends IssuableSubscriptionVisitor { - - @Override - public List nodesToVisit() { - return Collections.singletonList(Kind.SYNCHRONIZED_STATEMENT); - } - - @Override - public void visitNode(Tree tree) { - ExpressionTree expression = ((SynchronizedStatementTree) tree).expression(); - if (expression.symbolType().isSubtypeOf("java.util.concurrent.locks.Lock")) { - reportIssue(expression, "Synchronize on this \"Lock\" object using \"acquire/release\"."); - } - } - -} diff --git a/java-checks/src/main/java/org/sonar/java/checks/SynchronizedOnConcurrentObjectCheck.java b/java-checks/src/main/java/org/sonar/java/checks/SynchronizedOnConcurrentObjectCheck.java index d537b848621..3f9237e7f82 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/SynchronizedOnConcurrentObjectCheck.java +++ b/java-checks/src/main/java/org/sonar/java/checks/SynchronizedOnConcurrentObjectCheck.java @@ -25,8 +25,9 @@ import org.sonar.plugins.java.api.tree.ExpressionTree; import org.sonar.plugins.java.api.tree.SynchronizedStatementTree; import org.sonar.plugins.java.api.tree.Tree; +import org.sonar.plugins.java.api.tree.Tree.Kind; -@Rule(key = "S9141") +@Rule(key = "S2442") public class SynchronizedOnConcurrentObjectCheck extends IssuableSubscriptionVisitor { private static final String CONCURRENT_LOCKS_PREFIX = "java.util.concurrent.locks."; @@ -42,22 +43,24 @@ public class SynchronizedOnConcurrentObjectCheck extends IssuableSubscriptionVis "java.util.concurrent.TransferQueue"); @Override - public List nodesToVisit() { - return Collections.singletonList(Tree.Kind.SYNCHRONIZED_STATEMENT); + public List nodesToVisit() { + return Collections.singletonList(Kind.SYNCHRONIZED_STATEMENT); } @Override public void visitNode(Tree tree) { ExpressionTree expression = ((SynchronizedStatementTree) tree).expression(); - Type expressionType = expression.symbolType(); - if (isConcurrentSyncPrimitive(expressionType)) { + Type type = expression.symbolType(); + if (isSynchronizationPrimitive(type)) { reportIssue(expression, String.format( - "Use the \"%s\" API for synchronization instead of a \"synchronized\" block.", expressionType.name())); + "Use the \"%s\" API for synchronization instead of a \"synchronized\" block.", type.name())); } } - private static boolean isConcurrentSyncPrimitive(Type type) { - return isKnownSyncPrimitive(type) || type.symbol().superTypes().stream().anyMatch(SynchronizedOnConcurrentObjectCheck::isKnownSyncPrimitive); + private static boolean isSynchronizationPrimitive(Type type) { + return type.isSubtypeOf("java.util.concurrent.locks.Lock") + || isKnownSyncPrimitive(type) + || type.symbol().superTypes().stream().anyMatch(SynchronizedOnConcurrentObjectCheck::isKnownSyncPrimitive); } private static boolean isKnownSyncPrimitive(Type type) { diff --git a/java-checks/src/test/java/org/sonar/java/checks/SynchronizedLockCheckTest.java b/java-checks/src/test/java/org/sonar/java/checks/SynchronizedLockCheckTest.java deleted file mode 100644 index cb03810805f..00000000000 --- a/java-checks/src/test/java/org/sonar/java/checks/SynchronizedLockCheckTest.java +++ /dev/null @@ -1,33 +0,0 @@ -/* - * SonarQube Java - * Copyright (C) SonarSource Sàrl - * mailto:info AT sonarsource DOT com - * - * You can redistribute and/or modify this program under the terms of - * the Sonar Source-Available License Version 1, as published by SonarSource Sàrl. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. - * See the Sonar Source-Available License for more details. - * - * You should have received a copy of the Sonar Source-Available License - * along with this program; if not, see https://sonarsource.com/license/ssal/ - */ -package org.sonar.java.checks; - -import org.junit.jupiter.api.Test; -import org.sonar.java.checks.verifier.CheckVerifier; - -import static org.sonar.java.checks.verifier.TestUtils.mainCodeSourcesPath; - -class SynchronizedLockCheckTest { - - @Test - void test() { - CheckVerifier.newVerifier() - .onFile(mainCodeSourcesPath("checks/SynchronizedLockCheckSample.java")) - .withCheck(new SynchronizedLockCheck()) - .verifyIssues(); - } -} diff --git a/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9141.html b/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9141.html deleted file mode 100644 index b28d782336e..00000000000 --- a/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9141.html +++ /dev/null @@ -1,76 +0,0 @@ -

    This is an issue when using synchronized blocks on objects from the java.util.concurrent package, such as -ReentrantLock, Semaphore, CountDownLatch, CyclicBarrier, or BlockingQueue -implementations.

    -

    Why is this an issue?

    -

    Concurrent programming libraries provide their own high-level synchronization mechanisms. These are designed to be more flexible and powerful than -the language’s basic object locking mechanism.

    -

    When you use the basic locking mechanism on such an object, you are acquiring a lock that is built into the object itself at the language runtime -level. However, this built-in lock is completely separate from the object’s own synchronization protocol. The two mechanisms do not interact with each -other at all.

    -

    For example:

    -
      -
    • Using basic locking on a reentrant lock object does NOT call the object’s lock acquisition or release methods
      • -
    • Using basic locking on a semaphore object does NOT call the object’s acquire or release methods
      • -
    • Using basic locking on a countdown latch object does NOT call the object’s await or countdown methods
      • -
    -

    This means that threads using the basic locking mechanism on these objects will not coordinate with threads using the object’s proper API methods. -Different threads might think they have exclusive access when they actually don’t, leading to race conditions and data corruption.

    -

    This pattern typically indicates a misunderstanding of how concurrent library classes work. The developer likely intended to use the object’s own -synchronization mechanism but mistakenly used the basic locking mechanism instead.

    -

    In Java, this refers to classes from the java.util.concurrent package and using the synchronized keyword. Specific -examples include using synchronized(reentrantLock) instead of calling lock()/unlock() on a -ReentrantLock, or using synchronized(semaphore) instead of calling acquire()/release() on a -Semaphore.

    -

    What is the potential impact?

    -

    When language-level locking mechanisms are used on concurrent data structures that implement their own internal synchronization, the code fails to -provide the expected thread-safety guarantees:

    -
      -
    • Race conditions: Multiple threads may access shared resources simultaneously, even though the code appears to prevent this
      • -
    • Data corruption: Concurrent modifications to shared data can lead to inconsistent state
      • -
    • Logic errors: The application may behave incorrectly in multi-threaded scenarios, with bugs that are difficult to reproduce and - diagnose
      • -
    • False sense of security: The presence of explicit locking blocks may give developers and reviewers false confidence that the - code is thread-safe
      • -
    -

    These issues are particularly dangerous because they often only manifest under specific timing conditions, making them hard to detect during -testing.

    -

    How to fix it

    -

    Replace the synchronized block with the correct API methods for the specific java.util.concurrent class you’re using. -Always use try-finally blocks when acquiring locks to ensure they are released even if an exception occurs.

    -

    Code examples

    -

    Noncompliant code example

    -
    -private final ReentrantLock lock = new ReentrantLock();
-
-public void doWork() {
-    synchronized (lock) {  // Noncompliant
-        criticalSection();
-    }
-}
-
    -

    Compliant solution

    -
    -private final ReentrantLock lock = new ReentrantLock();
-
-public void doWork() {
-    lock.lock();
-    try {
-        criticalSection();
-    } finally {
-        lock.unlock();
-    }
-}
-
    -

    Resources

    -

    Documentation

    - -

    Related rules

    -
      -
    • {rule:java:S2445} - Blocks should be synchronized on "private final" fields
      • -
    - diff --git a/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9141.json b/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9141.json deleted file mode 100644 index c76a01f735d..00000000000 --- a/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9141.json +++ /dev/null @@ -1,26 +0,0 @@ -{ - "title": "Intrinsic locks should not be used on \"java.util.concurrent\" objects", - "type": "BUG", - "status": "ready", - "remediation": { - "func": "Constant\/Issue", - "constantCost": "10 min" - }, - "tags": [ - "concurrency", - "multi-threading", - "pitfall" - ], - "defaultSeverity": "Critical", - "ruleSpecification": "RSPEC-9141", - "sqKey": "S9141", - "scope": "All", - "quickfix": "unknown", - "code": { - "impacts": { - "RELIABILITY": "HIGH", - "MAINTAINABILITY": "MEDIUM" - }, - "attribute": "LOGICAL" - } -} diff --git a/sonar-java-plugin/src/main/resources/profiles/Sonar_way/S9141 b/sonar-java-plugin/src/main/resources/profiles/Sonar_way/S9141 deleted file mode 100644 index e69de29bb2d..00000000000