NX: Looking for comments on my lock/unlock methods

[Kerry Friesen]

Greetings,
Here are my lock and unlock methods. Could you provide some feedback on them please? Also, I'm not sure what to do with the InterruptedException in the lock method.
My assignment is URLyBird 1.3.1 and using a sockets implementation. I'm also having clients share one Data instance and intend on exposing its methods to the client directly - not providing a business layer. Is that considered a fat-client (2-tier implementation)?

    public void lock(int recNo) throws RecordNotFoundException
    {
        boolean valid = isValid(recNo);
        if(!valid)
        {
            String msg = "The specified record could not be found.";
            throw new RecordNotFoundException(msg);
        }
        synchronized(lockedRecords)
        {
            // Wait if the record is currently locked
            Integer key = new Integer(recNo);
            while(lockedRecords.containsKey(key))
            {
                try
                {
                    lockedRecords.wait();
                }
                catch(InterruptedException e)
                {
                    // What to do here?
                }
            }
            String name = Thread.currentThread().getName();
            lockedRecords.put(key, name);
        }
    }

    public void unlock(int recNo) throws RecordNotFoundException
    {
        boolean valid = isValid(recNo);
        if(!valid)
        {
            String msg = "The specified record could not be found.";
            throw new RecordNotFoundException(msg);
        }
        synchronized(lockedRecords)
        {
            Integer key = new Integer(recNo);
            String value = (String) lockedRecords.get(key);
            String name = Thread.currentThread().getName();
            if(name.equals(value))
            {
                lockedRecords.remove(key);
                lockedRecords.notifyAll();
            }
        }
    }

Regards,
Kerry

[Vlad Rabkin]

Hi Kerry,
1. Lock method:
You must check if the record still exists after getting the exclusive lock. Having it not done will lead to the deadlock. The record will be locked a one thread, but another thread could have deleted the record before.
2. Lock/Unlock I wouldn't use name of the thread. It is not going to work if you decide to switch your architecture to RMI later. This is design is not portable.

Best,
Vlad

[Andrew Monkhouse]

Hi Kerry,

Originally posted by Kerry Friesen:
Also, I'm not sure what to do with the InterruptedException in the lock method.

If you use the Search facility to search for InterruptedException you should find a few hints.
I think the consensus is that InterruptedException should not ever occur, since none of the code you write will ever try to interrupt your waiting threads. That being the case, if you get an InterruptedException, you may as well wrap it in a RuntimeException.

Originally posted by Kerry Friesen:
I'm also having clients share one Data instance

Since you are using Sockets and only using one Data instance, you should be OK using the name of the thread. As Vlad mentioned, this may cause you difficulties (not insurmountable difficulties though) if you use RMI later - but you can worry about that if the customer comes back and asks you to do that enhancement . You may want to mention it in your design decisions document though.

Originally posted by Kerry Friesen:
I also ... intend on exposing its methods to the client directly - not providing a business layer. Is that considered a fat-client (2-tier implementation)?

Yes - that is fat client / 2 tier implementation.
Regards, Andrew

[Kerry Friesen]

Thank-you Vlad and Andrew for your assistance... as always, very valuable help.
Cheers,
Kerry

[Philippe Maquet]

Hi Kerry,
I agree with Vlad said about checking for record existence after you get the lock.
Instead of storing and testing thread names, I would use thread references. Thread names are not garanteed to be unique while references are unique by definition.
Best,
Phil.