Merge pull request #20910 from yoff/java/more-thread-safe-initialisers

Author: yoff

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 field 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`.

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.

java/ql/test/query-tests/ThreadSafe/ThreadSafe.expected

@@ -43,3 +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: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 |

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<Integer, Integer> sync_map;
+    private final Map<Integer, Integer> sync_map_initialised = Collections.synchronizedMap(new HashMap<Integer, Integer>());
+
+    
+    private final Map<String, String> cmap;
+    private final Map<String, String> cmap_initialised = new ConcurrentHashMap();
+    private final Set<Integer> set;
+    private final Set<Integer> set_initialised = ConcurrentHashMap.newKeySet();
+    
+    public ThreadSafeInitializers() {
+        sync_map = Collections.synchronizedMap(new HashMap<Integer, Integer>());
+        cmap = new ConcurrentHashMap();
+        set = ConcurrentHashMap.newKeySet();
+    }
+
+    public void sync_map_put(Integer i, Integer v) {
+        sync_map.put(i,v);
+    }
+
+    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);
+    }
+
+    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);
+    }
+
+    public void set_initialised_add(Integer i) {
+        set_initialised.add(i);
+    }
+}