From 1ec947f27d77fb2894fdf0d736e7c1d8fc7b5c13 Mon Sep 17 00:00:00 2001 From: yoff Date: Tue, 25 Nov 2025 15:42:07 +0100 Subject: [PATCH 1/3] java: add tests for thread safe initialisation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Raúl Pardo --- .../ThreadSafe/ThreadSafe.expected | 4 ++ .../examples/ThreadSafeInitializers.java | 55 +++++++++++++++++++ 2 files changed, 59 insertions(+) create mode 100644 java/ql/test/query-tests/ThreadSafe/examples/ThreadSafeInitializers.java diff --git a/java/ql/test/query-tests/ThreadSafe/ThreadSafe.expected b/java/ql/test/query-tests/ThreadSafe/ThreadSafe.expected index 3d73caaffe56..c0df4852d780 100644 --- a/java/ql/test/query-tests/ThreadSafe/ThreadSafe.expected +++ b/java/ql/test/query-tests/ThreadSafe/ThreadSafe.expected @@ -43,3 +43,7 @@ | examples/Test.java:60:5:60:10 | this.y | This field access (publicly accessible via $@) is not protected by any monitor, but the class is annotated as @ThreadSafe. | examples/Test.java:60:5:60:10 | this.y | this expression | | examples/Test.java:74:5:74:10 | this.y | This field access (publicly accessible via $@) is not protected by any monitor, but the class is annotated as @ThreadSafe. | examples/Test.java:74:5:74:10 | this.y | this expression | | examples/Test.java:74:14:74:14 | y | This field access (publicly accessible via $@) is not protected by any monitor, but the class is annotated as @ThreadSafe. | examples/Test.java:74:14:74:14 | y | this expression | +| examples/ThreadSafeInitializers.java:29:9:29:16 | sync_map | This field access (publicly accessible via $@) is not protected by any monitor, but the class is annotated as @ThreadSafe. | examples/ThreadSafeInitializers.java:29:9:29:16 | sync_map | this expression | +| examples/ThreadSafeInitializers.java:37:9:37:12 | cmap | This field access (publicly accessible via $@) is not protected by any monitor, but the class is annotated as @ThreadSafe. | examples/ThreadSafeInitializers.java:37:9:37:12 | cmap | this expression | +| examples/ThreadSafeInitializers.java:45:9:45:14 | this.y | This field access (publicly accessible via $@) is not protected by any monitor, but the class is annotated as @ThreadSafe. | examples/ThreadSafeInitializers.java:45:9:45:14 | this.y | this expression | +| examples/ThreadSafeInitializers.java:49:9:49:11 | set | This field access (publicly accessible via $@) is not protected by any monitor, but the class is annotated as @ThreadSafe. | examples/ThreadSafeInitializers.java:49:9:49:11 | set | this expression | diff --git a/java/ql/test/query-tests/ThreadSafe/examples/ThreadSafeInitializers.java b/java/ql/test/query-tests/ThreadSafe/examples/ThreadSafeInitializers.java new file mode 100644 index 000000000000..8262c6bf69e7 --- /dev/null +++ b/java/ql/test/query-tests/ThreadSafe/examples/ThreadSafeInitializers.java @@ -0,0 +1,55 @@ +package examples; + +import java.util.Map; +import java.util.Set; +import java.util.HashMap; +import java.util.Collections; +import java.util.concurrent.ConcurrentHashMap; + +@ThreadSafe +public class ThreadSafeInitializers { + + private int y; + private final Map sync_map; + private final Map sync_map_initialised = Collections.synchronizedMap(new HashMap()); + + + private final Map cmap; + private final Map cmap_initialised = new ConcurrentHashMap(); + private final Set set; + private final Set set_initialised = ConcurrentHashMap.newKeySet(); + + public ThreadSafeInitializers() { + sync_map = Collections.synchronizedMap(new HashMap()); + cmap = new ConcurrentHashMap(); + set = ConcurrentHashMap.newKeySet(); + } + + public void sync_map_put(Integer i, Integer v) { + sync_map.put(i,v); // $ SPURIOUS: Alert + } + + public void sync_map_initialised_put(Integer i, Integer v) { + sync_map_initialised.put(i,v); + } + + public void cmap_put(String s1, String s2) { + cmap.put(s1, s2); // $ SPURIOUS: Alert + } + + public void cmap_initialised_put(String s1, String s2) { + cmap_initialised.put(s1, s2); + } + + public void setY(int y) { + this.y = y; // $ Alert + } + + public void set_add(Integer i) { + set.add(i); // $ SPURIOUS: Alert + } + + public void set_initialised_add(Integer i) { + set_initialised.add(i); + } +} \ No newline at end of file From 4687d09acb3a4305a2eb6c72741ab3e1330d3f7c Mon Sep 17 00:00:00 2001 From: yoff Date: Tue, 25 Nov 2025 15:46:18 +0100 Subject: [PATCH 2/3] java: understand more initializers Whne a fiels is assigned a safe type in a constructor, that field is not exposed. --- .../ql/lib/semmle/code/java/ConflictingAccess.qll | 15 +++++++++++++-- .../query-tests/ThreadSafe/ThreadSafe.expected | 3 --- .../examples/ThreadSafeInitializers.java | 6 +++--- 3 files changed, 16 insertions(+), 8 deletions(-) diff --git a/java/ql/lib/semmle/code/java/ConflictingAccess.qll b/java/ql/lib/semmle/code/java/ConflictingAccess.qll index ceff3e4ffa3a..128867690950 100644 --- a/java/ql/lib/semmle/code/java/ConflictingAccess.qll +++ b/java/ql/lib/semmle/code/java/ConflictingAccess.qll @@ -63,12 +63,23 @@ class ExposedField extends Field { not this.getType() instanceof LockType and // field is not thread-safe not isThreadSafeType(this.getType()) and - not isThreadSafeType(this.getInitializer().getType()) and + not isThreadSafeType(initialValue(this).getType()) and // the initializer guarantees thread safety - not isThreadSafeInitializer(this.getInitializer()) + not isThreadSafeInitializer(initialValue(this)) } } +/** + * Gets the initial value for the field `f`. + * This is either a static initializer or an assignment in a constructor. + */ +Expr initialValue(Field f) { + result = f.getInitializer() + or + result = f.getAnAssignedValue() and + result.getEnclosingCallable() = f.getDeclaringType().getAConstructor() +} + /** * A field access that is exposed to potential data races. * We require the field to be in a class that is annotated as `@ThreadSafe`. diff --git a/java/ql/test/query-tests/ThreadSafe/ThreadSafe.expected b/java/ql/test/query-tests/ThreadSafe/ThreadSafe.expected index c0df4852d780..488cfa1a482a 100644 --- a/java/ql/test/query-tests/ThreadSafe/ThreadSafe.expected +++ b/java/ql/test/query-tests/ThreadSafe/ThreadSafe.expected @@ -43,7 +43,4 @@ | examples/Test.java:60:5:60:10 | this.y | This field access (publicly accessible via $@) is not protected by any monitor, but the class is annotated as @ThreadSafe. | examples/Test.java:60:5:60:10 | this.y | this expression | | examples/Test.java:74:5:74:10 | this.y | This field access (publicly accessible via $@) is not protected by any monitor, but the class is annotated as @ThreadSafe. | examples/Test.java:74:5:74:10 | this.y | this expression | | examples/Test.java:74:14:74:14 | y | This field access (publicly accessible via $@) is not protected by any monitor, but the class is annotated as @ThreadSafe. | examples/Test.java:74:14:74:14 | y | this expression | -| examples/ThreadSafeInitializers.java:29:9:29:16 | sync_map | This field access (publicly accessible via $@) is not protected by any monitor, but the class is annotated as @ThreadSafe. | examples/ThreadSafeInitializers.java:29:9:29:16 | sync_map | this expression | -| examples/ThreadSafeInitializers.java:37:9:37:12 | cmap | This field access (publicly accessible via $@) is not protected by any monitor, but the class is annotated as @ThreadSafe. | examples/ThreadSafeInitializers.java:37:9:37:12 | cmap | this expression | | examples/ThreadSafeInitializers.java:45:9:45:14 | this.y | This field access (publicly accessible via $@) is not protected by any monitor, but the class is annotated as @ThreadSafe. | examples/ThreadSafeInitializers.java:45:9:45:14 | this.y | this expression | -| examples/ThreadSafeInitializers.java:49:9:49:11 | set | This field access (publicly accessible via $@) is not protected by any monitor, but the class is annotated as @ThreadSafe. | examples/ThreadSafeInitializers.java:49:9:49:11 | set | this expression | diff --git a/java/ql/test/query-tests/ThreadSafe/examples/ThreadSafeInitializers.java b/java/ql/test/query-tests/ThreadSafe/examples/ThreadSafeInitializers.java index 8262c6bf69e7..b8f50405066f 100644 --- a/java/ql/test/query-tests/ThreadSafe/examples/ThreadSafeInitializers.java +++ b/java/ql/test/query-tests/ThreadSafe/examples/ThreadSafeInitializers.java @@ -26,7 +26,7 @@ public ThreadSafeInitializers() { } public void sync_map_put(Integer i, Integer v) { - sync_map.put(i,v); // $ SPURIOUS: Alert + sync_map.put(i,v); } public void sync_map_initialised_put(Integer i, Integer v) { @@ -34,7 +34,7 @@ public void sync_map_initialised_put(Integer i, Integer v) { } public void cmap_put(String s1, String s2) { - cmap.put(s1, s2); // $ SPURIOUS: Alert + cmap.put(s1, s2); } public void cmap_initialised_put(String s1, String s2) { @@ -46,7 +46,7 @@ public void setY(int y) { } public void set_add(Integer i) { - set.add(i); // $ SPURIOUS: Alert + set.add(i); } public void set_initialised_add(Integer i) { From efb6a791970155848d524958292bda300f334db2 Mon Sep 17 00:00:00 2001 From: yoff Date: Tue, 25 Nov 2025 15:50:53 +0100 Subject: [PATCH 3/3] java: add change note --- .../src/change-notes/2025-11-25-thread-safe-initializers.md | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 java/ql/src/change-notes/2025-11-25-thread-safe-initializers.md diff --git a/java/ql/src/change-notes/2025-11-25-thread-safe-initializers.md b/java/ql/src/change-notes/2025-11-25-thread-safe-initializers.md new file mode 100644 index 000000000000..f373dae839df --- /dev/null +++ b/java/ql/src/change-notes/2025-11-25-thread-safe-initializers.md @@ -0,0 +1,4 @@ +--- +category: minorAnalysis +--- +* Java thread safety analysis now understands initialization to thread safe classes inside constructors. \ No newline at end of file