Logical locking - am i missing something?

[Margarita Babkova]

Hi there,

I have a question regarding my lock/unlock implementation it looks somewhat too simple – like I am missing huge chunk of possible issues.
I do not throw RNF exception - all this locking / unlocking should be done with conjunction of update / delete methods that will check record existence anyway.
I am going to implement RMI, and could NOT use threads and have to use unique client ID, that would be generated by RMI factory.
And that each client will have own instance of data class. ( I did not started RMI reading yet, so for me it is just a theory at this point).

The unique clientID somehow will be passed to LockManager class. How? – I do not know yet…
I saw that it is possible to use reference this instead of clientID.
I guess this is refers to the given instance of data, and as such uniquely identifies it.

[edit] do not post complete snippets of actual code

Thank you in advance,
M

[Roel De Nijs]

Your "logical locking" logic is completely messed up.

Some remarks:

If I try to lock a non-existing record, I expect a RNFE on the call to lock (not when calling update/delete).

You require a lock prior to updating/deleting a record, so checking for record existence is stupid (because if the record doesn't exist, you can't get the lock).

Do you know the difference between notify and notifyAll? What would be the best choice for unlock-method?

[Anayonkar Shivalkar]

Margarita Babkova wrote:I saw that it is possible to use reference this instead of clientID.

Yes. IMHO, it is better than unique clientID.

[Margarita Babkova]

I am, glad I asked.

I can easily add check for a RNF by making sure that requested recNo is in a main cache cache.containsKey(recNo) ?

As for the second point – think I am getting very close to understanding the difference between notify() and notifyAll(). And will use notifyAll() - now makes sense.

“The notify() method is generally used for resource pools, where there are an arbitrary number of "consumers" or "workers" that take resources, but when a resource is added to the pool, only one of the waiting consumers or workers can deal with it. The notifyAll() method is actually used in most other cases. Strictly, it is required to notify waiters of a condition that could allow multiple waiters to proceed. But this is often difficult to know. So as a general rule, if you have no particular logic for using notify(), then you should probably use notifyAll(), because it is often difficult to know exactly what threads will be waiting on a particular object and why.”

I am struggling with idea of this - what is really this - besides that is reference of object to itself - id is a real number so to say. But how do you measure reference this?

Thank you.

[Roel De Nijs]

Margarita Babkova wrote:I can easily add check for a RNF by making sure that requested recNo is in a main cache cache.containsKey(recNo) ?

Margarita Babkova wrote:I am struggling with idea of this - what is really this - besides that is reference of object to itself - id is a real number so to say. But how do you measure reference this?

I don't understand your question

[Margarita Babkova]

lockedRecordsMap.put(recNo, this);
and
lockedRecordsMap.put(recNo, ClientId);
I am trying explain to myself the difference between these two lines of code. I like first line more – there is no worries about getting ClientId to my data class. And in a sense it is the same: this refers to the given instance of Data class.
But will it lead to the problems down the road when I will start RMI implementation and have to create business method bookRoom( recNo)….

[Roel De Nijs]

I don't think your interface allows methods which take a reference to the Data class as parameter for lock/update/delete/unlock. The given interface only has methods with a clientId (int, long or String). So how are you going to work around this issue?

[Margarita Babkova]

Well, I only need to pass int RecNo to methods.
And then use this.toString as a value for the locking map.

I tried it in a simple class, and created several instances of it.
And on each called this.toString - it uniquely identified my instances. So, I think it could work with
RMI generated instances of my Data class.

Now, I am planning to use a Facade pattern, and wrap my lock methods with a calls to them that will thow this RNF. So, my LockManger class can get away from them (lock manager has no clue about main cache, or array list for my deleted records.

[edit] do not post actual code snippets

[Helen Ma]

Hi, let me analyze this code:

[edit] do not post actual code snippets

Hi, when I look at the code, it is equivalent to:

[edit] do not post actual code snippets

So, if I have Object o =new Object();
o.lock means it locks itself, and wait for itself forever.
Typically, we have this "pattern"

 public void lock(){
     Object o1=new Object();
       synchronize(o1){
               ....
           o1.wait();
      }
 }

If I have o.lock(), it means o locks o1, then release the lock of o1, and o will wait for o1 until other thread notifies o.
If o locks itself, release the lock of itself, and o waits for itself. Will o waits forever and cause deadlock?
Please help me to verify. I can be wrong with this analysis.

[Helen Ma]

OK.......I don't know the code that I quoted come from Margarita's actual work.

By the way, if I have this code:

      void methodA(){

              synchronized(this){
                     ....
                      wait();
             }
     }

Is there any possibility that the object that invokes methodA, wait for itself forever? This object locks itself and waits for itself.

[Roel De Nijs]

Margarita Babkova wrote:I tried it in a simple class, and created several instances of it.
And on each called this.toString - it uniquely identified my instances. So, I think it could work with
RMI generated instances of my Data class.

Always think about thread-safety! If you have multiple instances of your Data class (1 instance per client/thread thanks to the rmi factory) it's no use you synchronize on instance methods, because they will never be called by multiple threads (because each thread has its own instance of the class). If you change a member that's shared by all instances of your class (e.g. a static member) you need to synchronize on the static data member instead to have a thread-safe class.

[Roel De Nijs]

Helen Ma wrote:So, if I have Object o =new Object();
o.lock means it locks itself, and wait for itself forever.
Typically, we have this "pattern"

 public void lock(){
     Object o1=new Object();
       synchronize(o1){
               ....
           o1.wait();
      }
 }

If I have o.lock(), it means o locks o1, then release the lock of o1, and o will wait for o1 until other thread notifies o.
If o locks itself, release the lock of itself, and o waits for itself. Will o waits forever and cause deadlock?
Please help me to verify. I can be wrong with this analysis.

Why would you synchronize on a local variable that you just have created. Each thread has its own copy of o1, so why synchronize on that object? This case is a complete other story, but in your scenario I don't know why you would write such code.

[Roel De Nijs]

Helen Ma wrote:

      void methodA(){

              synchronized(this){
                     ....
                      wait();
             }
     }

Is there any possibility that the object that invokes methodA, wait for itself forever? This object locks itself and waits for itself.

That's something you can easily test yourself: just create a class containing this method, create an instance, invoke the method and see what happens.

[Helen Ma]

This is an experiment I did to cause a deadlock.
Step 1: Client B notify Client A. But Client A has not run the run() method yet. Client B's loop stops.
Step 2: Client A starts running run() method. Client A locks itself, and release its own lock and wait for gaining its own lock again.
It waits forever because Client B's synchronized block completes in step1.

I am not sure if this can answer Margarita's original issue about getting a deadlock.

public class ClientA extends Thread{
     
	public void run(){
		doIt();
	}
	private void doIt(){
		  synchronized(this){
			  while (true){ 
			  try{
				  System.out.println(" Client A waits ");
		           wait();
			   }
			  catch (InterruptedException e){
				  e.printStackTrace();
			   }
			  }
		  }
		 
	}
	public static void main(String...s){
		Thread clienta = new ClientA();
		Thread clientb = new ClientB(clienta);
		clienta.start();
		clientb.start();
		
	}
}

class ClientB extends Thread{
	Thread t =null;
	
	
	public ClientB(Thread t){
		 this.t=t;
	}
	
	public void run(){
		 doIt();
		
	}
	private void doIt(){
		 synchronized(t){
			 while ( true){ 
				  try{
					  
			           t.notify();
			           System.out.println("Notify Client A ");
			           break; 
				   }
				  catch (IllegalMonitorStateException e){
					  e.printStackTrace();
				   }
		    }//while
		 }//synchronize
		 
	} //doIt	
}