+
+
+
+
+In a thread-safe class, values must be published safely to avoid inconsistent or unexpected behavior caused by visibility issues between threads. If a value is not safely published, one thread may see a stale or partially constructed value written by another thread, leading to subtle concurrency bugs.
+
+
+In particular, values of primitive types should not be initialised to anything but their default values (which for Object is null) unless this happens in a static context.
+
+
+Techniques for safe publication include:
+
+
+- Using synchronized blocks or methods to ensure that a value is fully constructed before it is published.
+- Using volatile fields to ensure visibility of changes across threads.
+- Using thread-safe collections or classes that provide built-in synchronization, such as are found in
java.util.concurrent.
+- Using the
final keyword to ensure that a reference to an object is safely published when the object is constructed.
+
+
+
+
+
+
+Choose a safe publication technique that fits your use case. If the value only needs to be written once, say for a singleton, consider using the final keyword. If the value is mutable and needs to be shared across threads, consider using synchronized blocks or methods, or using a thread-safe collection from java.util.concurrent.
+
+
+
+
+
+In the following example, the values of value and server_id are not safely published. The constructor creates a new object and assigns it to the field value. However, the field is not declared as volatile or final, and there are no synchronization mechanisms in place to ensure that the value is fully constructed before it is published. A different thread may see the default value null. Similarly, the field server_id may be observed to be 0.
+
+
+
+To fix this example, we declare the field value as volatile. This will ensure that all changes to the field are visible to all threads. The field server_id is only meant to be written once, so we only need the write inside the constructor to be visible to other threads; declaring it final guarantees this:
+
+
+
+
+
+
+
+
+ Java Language Specification, chapter 17:
+ Threads and Locks.
+
+
+ Java concurrency package:
+ java.util.concurrent.
+
+
+
+
+
diff --git a/java/ql/src/Likely Bugs/Concurrency/SafePublication.ql b/java/ql/src/Likely Bugs/Concurrency/SafePublication.ql
new file mode 100644
index 000000000000..5e2a89ce3721
--- /dev/null
+++ b/java/ql/src/Likely Bugs/Concurrency/SafePublication.ql
@@ -0,0 +1,93 @@
+/**
+ * @name Safe publication
+ * @description A field of a thread-safe class is not safely published.
+ * @kind problem
+ * @problem.severity warning
+ * @precision high
+ * @id java/safe-publication
+ * @tags quality
+ * reliability
+ * concurrency
+ */
+
+import java
+import semmle.code.java.ConflictingAccess
+
+/**
+ * Holds if `v` should be the default value for the field `f`.
+ * That is, `v` is an initial (or constructor) assignment of `f`.
+ */
+predicate shouldBeDefaultValueFor(Field f, Expr v) {
+ v = f.getAnAssignedValue() and
+ (
+ v = f.getInitializer()
+ or
+ v.getEnclosingCallable() = f.getDeclaringType().getAConstructor()
+ )
+}
+
+/**
+ * Gets the default value for the field `f`.
+ * See https://docs.oracle.com/javase/tutorial/java/nutsandbolts/datatypes.html
+ * for the default values of the primitive types.
+ * The default value for non-primitive types is null.
+ */
+bindingset[result]
+Expr getDefaultValue(Field f) {
+ f.getType().hasName("byte") and result.(IntegerLiteral).getIntValue() = 0
+ or
+ f.getType().hasName("short") and result.(IntegerLiteral).getIntValue() = 0
+ or
+ f.getType().hasName("int") and result.(IntegerLiteral).getIntValue() = 0
+ or
+ f.getType().hasName("long") and
+ (
+ result.(LongLiteral).getValue() = "0" or
+ result.(IntegerLiteral).getValue() = "0"
+ )
+ or
+ f.getType().hasName("float") and result.(FloatLiteral).getValue() = "0.0"
+ or
+ f.getType().hasName("double") and result.(DoubleLiteral).getValue() = "0.0"
+ or
+ f.getType().hasName("char") and result.(CharacterLiteral).getCodePointValue() = 0
+ or
+ f.getType().hasName("boolean") and result.(BooleanLiteral).getBooleanValue() = false
+ or
+ not f.getType().getName() in [
+ "byte", "short", "int", "long", "float", "double", "char", "boolean"
+ ] and
+ result instanceof NullLiteral
+}
+
+/**
+ * Holds if all constructor or initial assignments (if any) are to the default value.
+ * That is, assignments by the declaration:
+ * int x = 0; OK
+ * int x = 3; not OK
+ * or inside a constructor:
+ * public c(a) {
+ * x = 0; OK
+ * x = 3; not OK
+ * x = a; not OK
+ * }
+ */
+predicate isAssignedDefaultValue(Field f) {
+ forall(Expr v | shouldBeDefaultValueFor(f, v) | v = getDefaultValue(f))
+}
+
+predicate isSafelyPublished(Field f) {
+ f.isFinal() or // NOTE: For non-primitive types, 'final' alone does not guarantee safe publication unless the object is immutable or safely constructed. Consider reviewing the handling of non-primitive fields for safe publication.
+ f.isStatic() or
+ f.isVolatile() or
+ isThreadSafeType(f.getType()) or
+ isThreadSafeType(f.getInitializer().getType()) or
+ isAssignedDefaultValue(f)
+}
+
+from Field f, ClassAnnotatedAsThreadSafe c
+where
+ f = c.getAField() and
+ not isSafelyPublished(f)
+select f, "The class $@ is marked as thread-safe, but this field is not safely published.", c,
+ c.getName()
diff --git a/java/ql/src/Likely Bugs/Concurrency/ThreadSafe.qhelp b/java/ql/src/Likely Bugs/Concurrency/ThreadSafe.qhelp
new file mode 100644
index 000000000000..cafcb3cdd799
--- /dev/null
+++ b/java/ql/src/Likely Bugs/Concurrency/ThreadSafe.qhelp
@@ -0,0 +1,33 @@
+
+