Managing Multiple Locks Problem

[Jerson Chua]

Hi Java Gurus...
I have implemented a lock manager which basically keep tracks of exclusive locks. I've found a bug but don't know how to fix it. Please help me if you know the solution. I am really desperate for some help.
Here's a part of my source code. The problem is inlined as comments.

public void lock(String resourceName) {
    ResourceLock lock = null;
    synchronized (mapsOfLocks) {
        lock = mapsOfLocks.get(resourceName);
        if (lock == null) {
            lock = new ResourceLock(resourceName);
            mapsOfLocks.put(resourceName, lock);    
        }
    }
    // what if the cleanup thread runs before the acquire() method is called
    // and the cleanup thread removes the lock from the HashMap
    // if this happen other thread would be able to lock the same resource
    // while the first thread still owns the lock cause a new instance
    // for the same resource will be created and stored in the HashMap.
    // I can't move this up in the synchronized block because of
    // nested-monitor lockout
    lock.acquire();
}
public void unlock(String resourceName) {
        ResourceLock lock = null;
        synchronized (mapOfLocks) {
            lock = (ResourceLock) locks.get(resourceName);
            /* just ignore lock attempts to unlocked resource */
            if ((lock == null) | | (!lock.isAcquired())) {
                return;
            }
            lock.release();
            
            if ((!lock.isAcquired()) 
                    && (!lock.isSomeoneWaiting())) {
                mapOfLocks.remove(lock.getResourceName());
            }
        }
}

// A thread that releases locks that have been locked for more than the 
// maximum allowable length of time. 
// It also removes the locks from the hash map if no one currently owns the 
// lock or are waiting to acquire the lock.
public void run() {
    while(true) {
        synchronized(mapOfLocks) {
                    Iterator iter = 
                ((Collection) mapOfLocks.values()).iterator();
            
            while(iter.hasNext()) {
                ResourceLock lock = (ResourceLock) iter.next();
                if (lock.hasTimedOut()) {                    
                    lock.release();
                    if ((!lock.isAcquired()) && (!lock.isSomeoneWaiting())) {
                                    iter.remove();
                    }
                }
            }
                }
            
        try {
            Thread.sleep(ResourceLock.LOCK_TIMEOUT / 5);
        } catch(InterruptedException e) {}
        }
}
 

[This message has been edited by Jerson Chua (edited March 14, 2001).]
Hi,


Please use [ code] [ /code] tags to format your code. Check out the UBB Code link below for further details



Thanks
[This message has been edited by Rahul Mahindrakar (edited March 14, 2001).]

[Rahul Rathore]

Jerson,
What was the need for a separate lock.acquire()? Would'nt it be better if put() into a mapOfLocks signifies the acquisition of locks? i.e. all locks put() in mapOfLocks must be acquired locks, otherwise they are not acquired. I think it is difficult to answer without seeing the rest of your code, and the purpose of you design? Which object calls lock() and for what purpose? What is the code of acquired()? What are the conditions under which a lock() is acquired and how/why are these conditions different from the conditions in which the lock is put() into the mapOfLocks.