3
votes

I need a simple lock, with timeout for JavaME (the backport of concurrent.lock needs full Java 1.3).

If someone else has already released tested locking code for JavaME, I would much rather use that.

Locking is notoriously difficult, so I thought I'd ask whether the following code is sane:

public class TimedLock {
    private volatile Thread holder = null;
    private Vector waiters = new Vector();

    public void lock(long ms) {
        synchronized (this) {
            if (holder == null) {
                holder = Thread.currentThread();
                return;
            }       
        }
        waiters.addElement(Thread.currentThread());
        try {
            Thread.sleep(ms);
            throw new RuntimeException("timeout while waiting for lock");
        } catch (InterruptedException e) {
            return;
        }
    }

    public synchronized void unlock() {
        if (holder != Thread.currentThread()) {
            throw new RuntimeException("attempting to release unheld lock");
        }
        // if there is at least one waiter, wake it 
        if (waiters.size() > 0) {
            holder = (Thread) waiters.elementAt(waiters.size() - 1);
            waiters.removeElementAt(waiters.size() - 1);
            holder.interrupt();
        } else {
            holder = null;
        }
    }
}
1

1 Answers

3
votes

You are developing an API. Do not synchronize on a public object.

If somebody instantiates one of your TimedLock and synchronizes on it, then it will stop working the way you expect it to.

TimedLock needs an internal private Object for its implementation to synchronize on.