+
+
+
+
+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 value of value is not safely published. The produce method
+ creates a new object and assigns it to the field value. However, the field is not
+ declared as volatile, and there are no synchronization mechanisms in place to ensure
+ that the value is fully constructed before it is published.
+
+
+
+To fix this example, declare the field value as volatile, or use
+ synchronized blocks or methods to ensure that the value is fully constructed before it is
+ published. We illustrate the latter with the following example:
+
+
+
+
+
+
+
+
+ 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..08cd3d5a5779
--- /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 // TODO: Consider non-primitive types
+ 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 @@
+
+