Skip to content

Commit 77734f8

Browse files
committed
java: better detection of thread safe fields.
Identified by triage of DCA results. Previously, we did not use the erased type, so would not recgnize `CompletableFuture<R>`. We now also recognize safe initializers.
1 parent bf13869 commit 77734f8

File tree

1 file changed

+13
-7
lines changed

1 file changed

+13
-7
lines changed

java/ql/lib/semmle/code/java/ConflictingAccess.qll

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -160,18 +160,22 @@ Class annotatedAsThreadSafe() { result.getAnAnnotation().getType().getName() = "
160160

161161
/** Holds if the type `t` is thread-safe. */
162162
predicate isThreadSafeType(Type t) {
163-
t.getName().matches(["Atomic%", "Concurrent%"])
163+
t.getErasure().getName().matches(["Atomic%", "Concurrent%"])
164164
or
165-
t.getName() in [
166-
"CopyOnWriteArraySet", "BlockingQueue", "ThreadLocal",
167-
// this is a method that returns a thread-safe version of the collection used as parameter
168-
"synchronizedMap", "Executor", "ExecutorService", "CopyOnWriteArrayList",
169-
"LinkedBlockingDeque", "LinkedBlockingQueue", "CompletableFuture"
170-
]
165+
t.getErasure().getName() in ["ThreadLocal"]
166+
or
167+
// Anything in `java.itul.concurrent` is thread safe.
168+
// See https://docs.oracle.com/javase/8/docs/api/java/util/concurrent/package-summary.html#MemoryVisibility
169+
t.getTypeDescriptor().matches("Ljava/util/concurrent/%;")
171170
or
172171
t = annotatedAsThreadSafe()
173172
}
174173

174+
/** Holds if the expression `e` is a thread-safe initializer. */
175+
predicate isThreadSafeInitializer(Expr e) {
176+
e.(Call).getCallee().getQualifiedName().matches("java.util.Collections.synchronized%")
177+
}
178+
175179
/**
176180
* A field access that is exposed to potential data races.
177181
* We require the field to be in a class that is annotated as `@ThreadSafe`.
@@ -185,6 +189,8 @@ class ExposedFieldAccess extends FieldAccess {
185189
// field is not thread-safe
186190
not isThreadSafeType(this.getField().getType()) and
187191
not isThreadSafeType(this.getField().getInitializer().getType()) and
192+
// the initializer guarantees thread safety
193+
not isThreadSafeInitializer(this.getField().getInitializer()) and
188194
// access is not the initializer of the field
189195
not this.(VarWrite).getASource() = this.getField().getInitializer() and
190196
// access not in a constructor

0 commit comments

Comments
 (0)