Concurrency Annotations: Getting started
Aaron Greenhouse
ago. I apologize for taking so long to do it. But I have a pretty
good excuse: my wife gave birth to our first child on April 25.
(Shameless promotion of cuteness: <http://www.cs.cmu.edu/~aaron/david/
>.)
(FYI, my background in this area is via the Fluid project at CMU,
which is now being commercialized by SureLogic, Inc. My dissertation
"A Programmer-Oriented Approach to Safe Concurrency" <http://reports-
archive.adm.cs.cmu.edu/anon/2003/abstracts/03-135.html> describes a
static analysis tool supported by annotations that assures programmer
consistency with programmer-stated concurrency design goals. The
scope of our annotations is probably beyond that which should be
considered for JSR-305, and are not limited to concurrency, although
they are currently tuned for that application. The interested reader
can get an idea of the Fluid annotations by looking at <http://
www.fluid.cs.cmu.edu:8080/Fluid/annotation-handout.html>.)
What follows is a bit disjoint. It is my initial thoughts and
reactions to the annotations that Brian proposes. It is intended to
kick start a long-needed discussion on concurrency annotations
Brian proposes four annotations:
@GuardedBy
@ThreadSafe
@NotThreadSafe
@Immutable
I'll start with the pair @ThreadSafe and @NotThreadSafe. At the
surface, these annotations are appealing, but in actuality they don't
convey a lot of information. That isn't to day they are useless,
however. The primary audience for @NotThreadSafe is not the analysis
tool, but the programmer reading the code. This is beneficial, of
course, because Java currently lacks a good way to explicitly indicate
that a class is not thread safe. Making it easy for the programmer to
keep this in mind prevents problems.
@ThreadSafe is not very helpful to implementors of the class because
it doesn't say anything about how the thread safety is accomplished,
and how it is to be maintained as the class is extended and evolved.
It is, however, useful to clients of the class. I developed an
annotation that I called @SelfProtected that is basically the same
thing. I wasn't able to assure anything about a class that is
annotated with @SelfProtected. I used it to suppress warnings in one
specific, but common situation: When you have a field access "o.f.g"
or a method invocation "o.f.m()" and it is known that field "f" of
object "o" is protected by a particular lock. In this case, the Fluid
tool issues a warning about the access to "g" or the invocation of
"m()", because even though the access to reference "o.f" can be
assured to be exclusive based on the locking information, we don't in
general know anything about the object referenced by "o.f." So we
warn that there is a possibility that a shared object is being
accessed even though a cursory inspection might lead the programmer to
believe all is well. But if the object referenced by "o.f" is of a
type that is annotated with @SelfProtected (aka @ThreadSafe), then the
warning is suppressed in the method call case because we get to assume
the method is implemented in such a way that it is thread safe with
respect to the rest of the methods in the type.
My one concern about @ThreadSafe annotation is that the "immature"
concurrent programmer may misinterpret it. It clearly doesn't
guarantee that a sequence of method invocations on a @ThreadSafe is
*atomic*. But I suspect many readers may believe that it is. I don't
really know a good solution to this problem other than providing very
clear documentation and examples about what kinds of scenarios do or
do not have guarantees. (This concern also applies to the @GuardedBy
annotation, but to a lesser extent because I think the @GuardedBy
annotation doesn't make as much of a blanket declaration of safety.)
@Immutable is interesting, but I think we have to make sure we really
stake down a definition of immutable. There are many competing ones
out there.
The meat of the Brian's proposal is @GuardedBy, which directly
associates a field of an object with the lock object that is meant to
protect it. It doubles as a mechanism for declaring lock
preconditions on methods. And, more subtly, it indirectly declares
that a method can return a specific lock. In Fluid, we support all of
these, although our notation is different because (1) we support
additional annotations for describing a more detailed hierarchical
model of the state of an object, and (2) we have an annotation to
explicitly declare a named lock. I think this level of sophistication
is beyond the scope of this standard, but my point is that we have
experience with this annotating exactly those things that Brian is
proposing. Happily, the domain of locks that Brian proposes exactly
matches that which we have also found to be necessary. (Although I'm
not entirely sure what Brian intends by the @GuardedBy("itself")
annotation.)
I think it is beyond argument that it is extremely useful to
explicitly indicate that relationship between state (fields) and
locks. Lock preconditions on methods are lesser used, but also quite
helpful. We have seen a real need for them in production code. Lock
getter methods I have found to be less useful in practice. The only
example I have seen in production code is the Component.getTreeLock()
method from the Java AWT. And this method by itself is quite useless
because the documentation doesn't tell you *when* you are supposed to
hold this lock. I've always argued that the lock getter method has
value because it allows the implementor to make the lock object
available to subclasses without having to reveal which object it
actually is. But, if we had to cut corners to save time, I think we
could safely discard support for lock getters and hardly anyone would
notice the difference.
There are a number of secondary implications to the @GuardedBy
annotation, particularly on the field that represents the lock. In
general, it should be final so that all users of the field actually
get a reference to the same lock. It should be our goal to enumerate
these constraints. A more general annotation-related question that
springs from this is what do we do about annotations that don't make
sense? That is, if we have a @GuardedBy("field1") annotation, but the
class doesn't have a field "field1"?
Brian's spec for @GuardedBy allows "ClassName.this" to be used a
lock. That is, the "outer receiver" can be used to lock fields in a
nested class. Fluid allows this too. We didn't originally, but we
encountered it on a case study of a commercial application. So it is
a useful piece of design intent to capture. But we did quickly
discover that it does, however, present a problem for static analysis
because it requires being able to track the identity of the "outer"
objects of an object:
pubilc class Outer {
private class Inner {
@GuardedBy("Outer.this")
public void method() { ... }
}
void doStuff(Inner in1, Inner in2) {
in1.method(); // [1]
in2.method(); // [2]
}
void evilMethod(Outer other) {
Inner in1 = this. new Inner();
Inner in2 = other. new Inner();
doStuff(in2, in1);
}
...
}
At line [1] we need to lock the object that was passed to the "other"
parameter of evilMethod(). At line [2] we need to lock the receiver
of doStuff() (because it was the receiver of evilMethod(). No local
information in method soStuff() provides us with this information.
Something to think about...
Brian Goetz
> I'll start with the pair @ThreadSafe and @NotThreadSafe. At the
> surface, these annotations are appealing, but in actuality they don't
> convey a lot of information.
I think they convey exactly one bit of information, but I'll agree that
they don't mean exactly the same bit that readers might think they mean.
@ThreadSafe is a statement of design intent; the creator of the class is
saying "My intention is that this class is safe to use from multiple
threads without additional coordination." Of course, the best of
intentions do not guarantee success. But this statement of intent is
useful to consumers of the code, to future maintainers as an indication
of requirements, and to tools which can help identify patterns of
questionable coding.
> @ThreadSafe is not very helpful to implementors of the class because
> it doesn't say anything about how the thread safety is accomplished,
> and how it is to be maintained as the class is extended and evolved.
Agreed, but by design. I believe that such information should be done
by other means, including but by no means limited to @GuardedBy.
Annotations are a pretty clunky mechanism; the more we try to push them,
the more unwieldy they get, and the less willing people are to use them.
"Marker" annotations like @ThreadSafe are probably the sweet spot in
terms of expressiveness.
Furthermore, I think there is not a sufficient literature of "thread
safety patterns" to make more detailed annotations (or variants of
@ThreadSafe, like @ThreadSafe(SelfGuardedMonitor) to be flexible enough
to cover the majority of code. But I'm willing to be convinced!
My overriding concern is that if we make the annotation set too
difficult to use, most users will not use it. And I think there's more
benefit in getting most people to use a simple but weak set of
annotations than in getting the experts to use a powerful but
complicated set of annotations.
> My one concern about @ThreadSafe annotation is that the "immature"
> concurrent programmer may misinterpret it. It clearly doesn't
> guarantee that a sequence of method invocations on a @ThreadSafe is
> *atomic*. But I suspect many readers may believe that it is. I don't
> really know a good solution to this problem other than providing very
> clear documentation and examples about what kinds of scenarios do or
> do not have guarantees.
You can put a warning label that says "Warning, do not point at people"
on a gun, but at some point you have to assume that your users have some
basic ability to not accidentally kill themselves or their neighbors.
One could argue that the Java threading model is fundamentally too
dangerous, but this is the world we live in. I fear worrying about
users who don't understand basic concepts of concurrency will be
paralyzing, so I've chosen to ignore it for the time being :)
One place where @ThreadSafe is too weak is that it only applies to
classes, not instances. But some Lists are thread-safe and some are
not. It would be nice to be able to say something like
@ThreadSafe Set s = ...
or
@Factory(@ThreadSafe.class)
public static Set synchronizedSet(Set s) {...}
This has more in common with the "type qualifier" sort of annotation,
like @NonNull, @Tainted, and friends.
> @Immutable is interesting, but I think we have to make sure we really
> stake down a definition of immutable. There are many competing ones
> out there.
Yes. For example, String caches the hashCode, but it is pretty
important that String be considered immutable.
> (Although I'm
> not entirely sure what Brian intends by the @GuardedBy("itself")
> annotation.)
This was one that I think I withdrew from my proposal before the book
was published, but may have leaked through into the code posted on the
web. The idea there was an state variable that was an object that is
guarded by its own intrinsic lock. For example:
@GuardedBy("itself") List list = new ArrayList();
...
synchronized (list) {
// do stuff with list
}
This is a pattern that shows up often enough, but @GuardedBy("itself")
is a bad name for it, because all the other arguments to @GuardedBy are
valid Java expressions that might appear as the lock of a synchronized
block.
> I think it is beyond argument that it is extremely useful to
> explicitly indicate that relationship between state (fields) and
> locks. Lock preconditions on methods are lesser used, but also quite
> helpful. We have seen a real need for them in production code. Lock
> getter methods I have found to be less useful in practice.
It shows up in real code, though arguably it is overused. It tends to
show up in abstract classes where the base class wants to let the
subclass choose the locking protocol.
> The only
> example I have seen in production code is the Component.getTreeLock()
> method from the Java AWT. And this method by itself is quite useless
> because the documentation doesn't tell you *when* you are supposed to
> hold this lock.
It shows up in the NIO provider interface too.
> There are a number of secondary implications to the @GuardedBy
> annotation, particularly on the field that represents the lock. In
> general, it should be final so that all users of the field actually
> get a reference to the same lock. It should be our goal to enumerate
> these constraints. A more general annotation-related question that
> springs from this is what do we do about annotations that don't make
> sense? That is, if we have a @GuardedBy("field1") annotation, but the
> class doesn't have a field "field1"?
My intention was for the argument of GB to be a Java expression. So
annotation processors should be able to parse it and then make
inferences like this one -- field does not exist, field is not final, etc.
Dave Griffith
>> springs from this is what do we do about annotations that don't make
>> sense? That is, if we have a @GuardedBy("field1") annotation, but the
>> class doesn't have a field "field1"?
>My intention was for the argument of GB to be a Java expression. So
>annotation processors should be able to parse it and then make
>inferences like this one -- field does not exist, field is not final, etc.
The LockSmith tool from Sixth and Red River Software already does
this, as an IntelliJ IDEA plugin. It can also automatically create
some JCiP annotations as part of it's concurrency-oriented program
transformations. If such checks aren't already part of FindBugs, I
can't imagine it would take much to add them.