more locking

[Goran Borozan]

some more questions:

1. is this locking algorithm ok :

public synchronized long lock(int recNo) throws RecordNotFoundException {
 
        Long lock = null;
 
        synchronized (locks) {
 
            //Wait until locks no longer contain lock on recNo
            while (locks.contains(new Integer(recNo))) {
                try {
                    locks.wait();
                } catch (InterruptedException e) {
                }
            }
 
            //Try to get record. Possible deleted. Will throw exception.
            try {
                getRecord(recNo);
            } catch (RecordNotFoundException e) {
                locks.notifyAll();
                throw e;
            }
 
            //Insert lock into locks
            lock = new Long(getNextCookie());
            locks.put(new Integer(recNo), lock);
 
            locks.notifyAll();
        }
 
        return lock.longValue();
    }

2. if i have tiis metod in my Data class how can i do with local mode (RMI-no locking needed) ?
3. Is it OK to declare locks private static in my Data class:
private static final Hashtable locks;

[Andrew: put the source code between [code] and [/code] UBB tags]
[ May 27, 2004: Message edited by: Andrew Monkhouse ]

[Goran Borozan]

Many tanks for all help i can get!!!
/Goran

[Baruch Sadogursky]

One question - why to notify after adding cookie to the HashTable? How adding the lock to the table can change the waiting condition of the waiting threads? Correct me if I am wrong, but the only place in which the waiting condition can change is in the unlock() method - so it's the place to notify.

[Jon Entwistle]

I would be careful about posting so much code if I were you - there is a danger that someone else will copy your code (and its mistakes) directly.

I see dangers fpr your threads - nested synchronisation and/or synchronizing the same data structure on more than one object is inviting problems.

Why do you notifyAll?

Jon

[Goran Borozan]

Tanks!
I understand that i dont need notifyAll in my lock method.
I will remove it.

What is the best way to separate rmi and local mode in the Data class?
(How can i avoid lockin in local mode ?)
I suppose that i must have locking in the Data class because Sun is testing this class.If i have a LockManager and use it from the DataImpl
the Sun tests will not pass.

Many tanks!
/Goran

[Andrew Monkhouse]

Hi Goran,

Welcome to JavaRanch and this forum.

I have edited your post to put the code between [code] and [/code] UBB tags. Doing this ensures that indenting is preserved, which makes the code easier to read.

When you are writing your post, there are a number of buttons just below the edit pane, which will insert the tags for you, so you don't have to remember what they are.

If you would like to edit your original post so that you can see what I have done, you can click on the button that is just above your post.

To clarify what Jon said, we try to limit the amount of code that is posted in this forum. However posting one method (like you have done) is fine. Our policy is described in the question "What is the policy on posting questions I saw on the exam / details of how to do the assignment?" in the JavaRanch SCJD FAQ.

What is the best way to separate rmi and local mode in the Data class?
(How can i avoid lockin in local mode ?)

You can't.

Your method signatures require the use of the lock cookie. Therefore, even in local mode, you will still need the lock cookie in order to call the update method. Therefore you must still lock records even in single user mode.

While I agree that locking is not needed in single user mode, using the same locking logic allows reuse of code - always a good thing. And, as I said before, with the method signatures you have been given, you can't avoid it .

Note that the Hashtable is synchronized, but you are calling it within a synchronized block. Therefore (as Jon mentioned) you are nesting synchronized blocks (implicitly, not explicitly) - this is a bad thing. Plus, you do not need the synchronization that Hashtables provide, so you may be better off looking at a different Collection object.

You have a lot of code inside your synchronized block. In general you want your synchronized block to be as small as possible. You might want to consider whether any of that code can be moved outside of the synchronized block.

You also have a case where you swallow the InterruptedException (catch it, but don't do anything with it) - this is generally considered a bad idea. You really should do something with that exception.

Regards, Andrew

[Jon Entwistle]

You are also synchonising your method on (this) while your nested block is synchronized on (lock) - Do you really need this? I would say that you are risking a deadlock with no gain.

Cheers,

Jon

[Jon Entwistle]

posting one method (like you have done) is fine. Our policy is described in the question "What is the policy on posting questions I saw on the exam / details of how to do the assignment?" in the JavaRanch SCJD FAQ.

Sorry Goran - I didn't know this was on the FAQ!

Jon
[ May 28, 2004: Message edited by: Jon Entwistle ]

[Philippe Maquet]

Hi Jon,

I would say that you are risking a deadlock with no gain.

In case the unlock method is also synchronized (I guess it's the case), a deadlock will even happen for sure the first time a thread will have to wait for a lock:

When a thread (thread A) starts waiting on locks, it releases its lock on locks, but keeps its lock on this. To be notified, the notifying thread (thread B) must first grab the lock on locks. But as unlock is synchronized, it must first grab the lock on this, lock which thread A still owns... = deadlock.

Regards,

Phil.