About thread safe...

[Sandra Baker]

(Edited the original post.....)
I used this data structure to store my locked record number, do I need to synchronize the object?
private static SortedSet lockedRec = Collections.synchronizedSortedSet(new TreeSet());
Here is how it works:
Clients need to check if the record number is currently in the lockedRec, if not, can lock the record. If yes, use while loop to try certain amount of times ( i set it to be 100 sec), after this amount of time, there may be something wrong with the client who's currently locking this record, then, explicitly unlock the record by the server.
[ October 09, 2002: Message edited by: Sandra Baker ]

[Max Habibi]

Hi Sandra,
You're almost there: Everything is right, except for the following:
1) you need to explicitly synchronize on your SortSet in the lock/unlock methods, using a while loop: for example...

public void lock(){
synchronized(mySortedSet){
//stuff
SortedSet.notifyAll();
}

2. Given the above, there is absolutely no need to use a synchronized collection.
I seem to recall that you said you had purchased my book. I thought I laid this out pretty explicitly there. Didn't I? I hope it doesn't sound like I'm chastising you: I just want to make sure I was clear.
All best,
M, author
The Sun Certified Java Developer Exam with J2SE 1.4
[ October 09, 2002: Message edited by: Max Habibi ]

[Mark Spritzler]

Clients need to check if the record number is currently in the lockedRec, if not, can lock the record. If yes, use while loop to try certain amount of times ( i set it to be 100 sec), after this amount of time, there may be something wrong with the client who's currently locking this record, then, explicitly unlock the record by the server.

Doesn't quite make sense to me here. Why have a time limit or a limited number of loops, that is not in the requirments. So then you automatically unlock the record just because it is possible that there is something wrong with the client?
All you need is a HashSet to store the locked records. when you call lock amke sure you use synchronize(<<HashSet>> {
}
and have your code try to get the lock using a while loop. and issueing a wait() if the record is locked by another user.
Then use the Unreferenced interface to handle your possible something wrong with the client issue, where in the Unreferenced method you unlock the records that client has.
This is nice and clean there is complete Thread Safety there in the HashSet when you synchronize on it, and wait if it is already locked.
You will find many posts on this approach, and that will get you the full amount of points.
Petitioning, unfortunately won't help in this case. Sorry.
Mark

[Max Habibi]

I just reread your post:
Mark is absolutely right, drop this whole business of looping to a hundred.
Here's how synchronization works. When you say

1: synchronized(myObj){
2:
3: while (myObj.contains(num) && !isOwner(num)) myObj.wait()
4: 
5: myObj.notifyAll();
6: }

what you're saying on line 1 is: "Hey, only let a single given thread -call him LuckyThread- use myObj right now. Tell all other Threads-Call them UnLuckyThreads- that want to use myObj to cool their heels for a bit.".
So now LuckyThread starts to execute, and he reaches line number 3. Say LuckyThreadwants to put 99 into myObj, but MyObj already has a 99. So what does LuckyThread do? He waits. He say "Geez, I can't do what I wanted to, so why don't you guys(UnLuckyThreads- ) enter a lottery and see who gets to be the next LuckyThread?". The unLuckys do, and a new LuckyThread is elected. Say she wants to remove 99 from myObj.
When the new LuckyThread reaches line 5, she says to all of those unlucky Threads. "Ok, I'm done with myObj now, and I'm outta here. Why don't you guys enter a lottery and see who gets to be the next LuckyThread?". And that's what those UnLucky threads do. So we get a new LuckyThread, and the cycle starts again.
All best,
M, author
The Sun Certified Java Developer Exam with J2SE 1.4
[ October 09, 2002: Message edited by: Max Habibi ]

[Max Habibi]

I hope that clear it up. If not, you're welcome to drop down to your local BN and read chapter 4: it should only take an hour or so. If they give you any trouble, tell'em I said it was ok .
All best,
M, author
The Sun Certified Java Developer Exam with J2SE 1.4

[christy smile]

Hi, Mark, Max
I have a question about synchronized and HashMap vs. HashSet.
I made my lock() and unlocked() synchroinzed functions so that I don't have to worry about synchronizing the HashMap I used to store the recordNumber and the corresponding object that has locked that record. Is this the right approach as well?
In addition, I chose HashMap instead of HashSet to store the record and object pair. Is there is big advantage of using HashSet? Or which one is better?
Thanks,
Christy

[Max Habibi]

Christy,
Just because your lock method is synchronized doesn't mean that you're threadsafe. Remember, since the HashMap is static, it's not an instance variable. That means the several distinct threads could still modify at the same time. Since you've decided to change the method signatures anyway, you're better off making lock static synchronized: then this issue goes away.
all best,
M, author
The Sun Certified Java Developer Exam with J2SE 1.4

[Mark Spritzler]

Like Max said, the best way to secure threadsafety for your HashSet, is to synchronize on it, and then check for the locked record in the HashSet in the while statement, if it is there
issue a wait(), having a Try Catch around it, catching the InterruptedException, and going about your businesss, cause now you can call put to the HashSet and lock the record for the client. then the synchronization section is ended.
Mark

[Venita Glasfurd]

and then check for the locked record in the HashSet in the while statement

Why check for a locked record in a while statement? I have used an if statement to check for a locked record. My implementation goes like this.

 
            synchronized(myLockTable)
            {
                if (myLockTable contains record number to be locked)
                {
                   wait
                }
                put record number in myLockTable
            }    
 

I cannot visualise a situation where a while loop is required. Am I missing something obvious here?
Venita

[Max Habibi]

It's not obvious, but there's a good reason to use a while statement.
It has to do with the way that while statements interact with Threads. Even in a synchronized block, your thread can be switched out.
However, when your thread switches back in, a while-condition is immediately check again, first thing. An if-statement is not. That why a block like

1: while(condition){
2:     wait();
3: }

is valid. It gets tested every time the thread slices in. An if-statement just does not.
HTH,
M, author
The Sun Certified Java Developer Exam with J2SE 1.4

[Venita Glasfurd]

Thanks, Max. I've got it.
Venita.

[christy smile]

Hi, Mark, Max,
Thank you for your comments about synchronization. I have another question about the lock()/unlock. For my current implementation, I have a DataLockManager associated with every remote client (I do not have any locking mechanism for the local client). In DataLockManager, I have a function called lockDB, which sets the "dblock" flag, similar to the following quote I found the in a previous posting:

You can do it either way. There are two schools of thought on locking the whole database: loop through all the records and lock them (this was Mark's approach); set a "dblock" flag, that when set, will not allow any further records to be locked and wait for all pending locks to be unlocked at which time the dblock is established (this was my and I believe Nate's and Eugene's approach).

My question is:
(1) Should I make the flag dblock static? Since I have multiple instances DataLockManager, but if one client locks the whole database, every client should know about it.
(2) Someone suggested in a different thread that the LockManager should be made singleton. Is this necessary if I have both my HashMap and the flag static?
(3) This one goes back to the synchronization issue. If I want ot synchronize on only the object, don't I need to synchronize both the HashMap and "dblock" flag in my lock()? The following is the code for my current lock() in DataLockManager (I have not changed the synchronization yet). By the way, I am bypassing the lock()/unlock in Data class completely. Do you think that's okay? The ClientId is the connection object.

 
  public synchronized void lock(String clientID, int recordNo) throws RemoteException, IOException
    {
        if ( recordNo < -1 || recordNo > mDB.getRecordCount() )
            throw new IOException("Record Does Not Exist");
        //locks the whole database
        if ( recordNo == -1 )
        {
            lockDB();
            return;
        }
        Integer recObj = new Integer(recordNo);
        String lockClientID = (String)mLockRecords.get(recObj);
        //the record has been locked by the same client, do nothing
        if ( lockClientID!=null && lockClientID.equals(clientID) )
            return ;
        //lock the record by placing the clientId and recordNo into the hashtable
        if ( lockClientID == null && !mDBLocked )
        {
            mLockRecords.put( recObj, clientID);
        }
        else
        {
            while ( mLockRecords.get( recObj ) != null || mDBLocked )
            {
                try
                {
                    wait();
                }
                catch ( Exception e )
                {
                    throw new IOException (UNEXPECTED + e.getMessage());
                }
            }
            mLockRecords.put( recObj, clientID );
        }           
    } 

Thanks.
Christy

[christy smile]

By the way, mDBLocked is the "dblock" flag and mLockRecord is the static HashMap. Currently, the mDBLocked is just a boolean variable. I am thinking about changing it to static as well.
Comments welcome!
Christy

[Daniel Chen]

Hi Max,
Why need notifyAll at the end of your method?
[ October 09, 2002: Message edited by: Daniel Chen ]

[Max Habibi]

Hi Danial,
NotifyAll tell every Thread that is waiting that the resource in now free.
HTH,
M, author
The Sun Certified Java Developer Exam with J2SE 1.4

[Mark Spritzler]

Christy you are very close.
"static? Since I have multiple instances DataLockManager"
No you do not need to make it static and I suggest you don't. You should only have one instance of the DataLockManager, and it shouldn't be a Singleton though. Like you create one instance of the Data class, you will also have only one LockManager created, which every Client Remote Object should have a reference too, not a seperate instance created in each.
That actually should answer all three of your questions.

[Sandra Baker]

Here is my lock and unlock.... please give me some feedback, I am kinda skeptical about two synchronized (myobj) in the lock...Thanks

  
//myObj stores the locked record numbers
private static Set myObj = Collections.synchronizedSet(new HashSet());
public void lock(recordNum) throws RemoteException, IOException{
try{
  synchronized(myObj){
     while(myObj.contains(recordNum) == true){
     //notifyAll, release the monitor of the object for others
    //wait
     }
   }
}catch (InterruptedException ie){System.out.println(ie);}
}
synchronized(myObj){
  //add record
  //notifyAll
}
public void unlock(recordNum) throws RemoteException{
  synchronized(myObj){
    //remove record
    //notifyAll
  }
  
} 

[ October 10, 2002: Message edited by: Sandra Baker ]

[Max Habibi]

You're very, very close. However, you can't just call wait: you need to call wait on myObj.
here's why:
When you wait(), you are, of course, really calling this.wait(). But the this object isn't the one you're synchronizing on: you're synchronizing on myObj. I've posted on this before, in a great deal of detail, so you might want to do a search on my posts if you still have some questions.
The same is true for notifyAll().
All best,
M, author
The Sun Certified Java Developer Exam with J2SE 1.4

[christy smile]

Hi, Mark, Max,
Thank you for the reply. I went back to my lock() and unlock() again just to familiarize myself (I did most of the SCJD coding about 6 months ago, and slept on it for almost half year, and determined finally to finish the exam by the end of this month hopefully. For the ranchers out there, please don't laught me at this ). Anyway, I realized that I misrepresented something in the previous post.
For each Data Server, there is actually only one DataLockManager instance associated (no matter how many clients are connected with the server).
Question #1: Given that, I don't even need to make my HashMap that stores the recordId and clientId pair static anymore, right?

You should only have one instance of the DataLockManager, and it shouldn't be a Singleton though. Like you create one instance of the Data class, you will also have only one LockManager created, which every Client Remote Object should have a reference too, not a seperate instance created in each.

[B]Question #2: Did not quite understand the above quote. I should only have one DataLockManager for each DataServer I create (each DataServer is in term associated with a Data file), right? But what if I have multiple data servers started with the same data file, I still only should have one DataLockManager associated with that Data file. Then, how am I going to keep track?
As far as my current implementation goes, if server#1 and Server#2 decided to serve the same database, then, DataLockManager#1 and DataLockManager#2 is created respectively. If the HashMap is static, that's okay. But I am thinking about changing the HashMap to just a member variable. Since if it is static, what if Server#1 and Server#2 started on different data file, the HashMap will be shared by different data files, which is not correct at all. What do you think about that?
[B]Question #3, if both HashMap and the dblocked flag are not static, should I use synchronized on the whole function? What is the advantage of only synchronizing on the HashMap. In addition, in my code where i have "wait()", according to Max's comment, shouldn't I use "mLockedRecord.wait()" as well if I just synchronize on mLockedRecord.
I hope I am making sense here. Thanks.
Christy