GuardedBy by ReadWriteLock

388 views
Skip to first unread message

Howard

unread,
Jun 26, 2008, 5:57:41 PM6/26/08
to JSR-305: Annotations for Software Defect Detection
Hello,

I'm looking for clarification on the @GuardedBy syntax.
Given the following code, what should be the argument to @GuardedBy
be?
Thanks!

package com.vaultus.server;

import java.util.concurrent.locks.Lock;
import java.util.concurrent.locks.ReadWriteLock;
import java.util.concurrent.locks.ReentrantReadWriteLock;

import net.jcip.annotations.GuardedBy;

public class Foo {
private final Lock m_readLock;
private final Lock m_writeLock;

@GuardedBy("?")
private int m_val;

public Foo() {
ReadWriteLock lock = new ReentrantReadWriteLock();
m_readLock = lock.readLock();
m_writeLock = lock.writeLock();
}

public void set(int val) {
m_writeLock.lock();
try {
m_val = val;
} finally {
m_writeLock.unlock();
}
}

public int get() {
m_readLock.lock();
try {
return m_val;
} finally {
m_readLock.unlock();
}
}
}

- Howard

Brian Goetz

unread,
Jul 1, 2008, 12:13:05 AM7/1/08
to jsr...@googlegroups.com
While @GuardedBy was not designed explicitly for read-write locks, I would say
the natural extension would be @GuardedBy("lock"), where "lock" is a
read-write lock.

In the case below, you'd have to refactor to not throw away the lock object.
Then, a static analyzer could determine that m_val is only touched with either
the reader or writer locks of "lock" held. (In the general case, the analyzer
would have to know what methods or operations are mutative to determine if the
correct orientation of lock is held for the operation.)

Aaron Greenhouse

unread,
Jul 1, 2008, 1:03:11 PM7/1/08
to JSR-305: Annotations for Software Defect Detection


On Jul 1, 12:13 am, Brian Goetz <br...@briangoetz.com> wrote:
> While @GuardedBy was not designed explicitly for read-write locks, I would say
> the natural extension would be @GuardedBy("lock"), where "lock" is a
> read-write lock.

I agree. The equivalent annotations what we currently use in
SureLogic JSure do this. Where ever we would use a "java.lang.Object"
as a lock, we can also accept a "java.util.concurrent.locks.Lock" or a
"java.util.concurrent.ReadWriteLock".


> In the case below, you'd have to refactor to not throw away the lock object.
> Then, a static analyzer could determine that m_val is only touched with either
> the reader or writer locks of "lock" held.

Agreed. JSure would require this change.

> (In the general case, the analyzer
> would have to know what methods or operations are mutative to determine if the
> correct orientation of lock is held for the operation.)

Brian, I think you are over-complicating the answer here. Really the
analyzer just has to understand field read (o.f) and field assignment
(o.f = ...).

As you know, it depends on where you draw the line about lock
responsibility. I think we all agree that the normal line is that
unless annotated otherwise, a method acquires (and releases!) all the
locks it needs during execution. So you normally don't have to worry
about what lock you need when calling a method. In this case, the
only operations are field assignment and field read, with the obvious
locking semantics. A method that doesn't acquire the necessary lock
needs its own @GuardedBy annotation. The only thing the analyzer
needs to know here is which field writes occurred without a lock,
which field reads occurred without a lock, and then to map those
fields to the locks they are supposed have. No explicit effect system
is required. (Of course, an explicit one is certainly useful for this
and for other things.)

Reply all
Reply to author
Forward
0 new messages