Record locking on recNo

[conny pemfors]

Hi.

According to the description of Data class.
Locks a record so that it can only be updated or deleted by this client.
If the specified record is already locked, the current thread gives up
the CPU and consumes no CPU cycles until the record is unlocked.

you should use record locking on recNo probably also on the client using the method lock(int recNo);

in my case I have done like in the dvd example,
using

        Long offset = 0L;
        recordNumbersLock.readLock().lock();
        try {
            offset = recordNumbers.get(record.getId()) ;
        } finally {
            recordNumbersLock.readLock().unlock();
        }
 
        if ( offset == null) {
            recordNumbersLock.writeLock().lock();

maybe I should redo this part with using a similar method like this in the file access class,

 
 void setDatabaseLockedRecord(int recNo) throws RecordNotFoundException{
        
        
        log.entering("RecordFileAccess", "setDatabaseLockedRecord", recNo);
        // Implementation details: any modification (create/update/delete) to
        // the database locks <code>recordNumbersLock</code> first, since the
        // modification might change the recordNumbers collection. Therefore
        // all we need do is lock this mutual exclusion lock, and no other
        // thread can modify it.
        try
        {
            Record record = this.getRecord(recNo);
            if( record == null || record.getId() == 0){
                throw new RecordNotFoundException("Record not found");
            }
            else{
                
                  recordLock.writeLock().lock();
                  
                
            }
        }catch(IOException e ){
                  log.throwing("RecordFileAccess","setDatabaseLockedRecord" , e);
  
        }

Conny

[Conny - I have edited your post to put code between [code] and [/code] UBB tags. This helps keep the code readable. If you would like to see what I have done, please click on the icon that appears at the start of your post. Regards, Andrew]
[ November 22, 2007: Message edited by: Andrew Monkhouse ]

[Alex Belisle Turcot]

Hi,

I'm not quite sure what is your question, could you rephrase for me ?

Regards,
Alex

[conny pemfors]

Originally posted by Alex Belisle Turcot:
Hi,

I'm not quite sure what is your question, could you rephrase for me ?

Regards,
Alex

According to the Data class you should use recNo.

 
 /**
     *  Locks a record so that it can only be updated or deleted by this client.
     *  If the specified record is already locked, the current thread gives up
     *  the CPU and consumes no CPU cycles until the record is unlocked.
     *
     *
     * @param int recNo lock specified record
     * @return void
     * @throws RecordNotFoundException
     */ 
    public void lock(int recNo) throws RecordNotFoundException;
 
     /**
     * Releases the lock on a record.
     *
     * @param int recNo unlock a specified record with recordId
     * @return void
     * @throws RecordBotFoundException
     */ 
    
    public void unlock(int recNo) throws RecordNotFoundException;
 
    
 
     /**
     * Determines if a record is currenly locked. Returns true if the
     * record is locked, false otherwise.
     *
     *
     * @param int recNo to validate if record with specified record id is locked
     * @return boolean returns true or false
     * @throws RecordNotFoundException, if record not found
     */ 
    public boolean isLocked(int recNo) throws RecordNotFoundException;

however in the dvd example they do not lock database record with recordNo.
I wonder how this should be implemented or if you just have to add record lock with recNo from GUI before doing update, insert or delete.


[Conny - I have edited your post to put code between [code] and [/code] UBB tags. This helps keep the code readable. If you would like to see what I have done, please click on the icon that appears at the start of your post. Regards, Andrew]
[ November 22, 2007: Message edited by: Andrew Monkhouse ]

[Musab Al-Rawi]

Hi Conny,

As an advice start with the Database first and forget about the GUI.
now in the DVD Example he is identifying the locker using an instance of the fasade (that's why he is sending "this" with the record number to the reservation manager instance).

You can use the same example in the DVD example. But make sure that your interface doesn't require you to return a magical number or cookie.

now you can improve on the design of the locking that you have in the book and most probably you have to make changes to suit your design and data.

hope that helped.
[ November 15, 2007: Message edited by: Musab Al-Rawi ]

[conny pemfors]

Originally posted by Musab Al-Rawi:
Hi Conny,

As an advice start with the Database first and forget about the GUI.
now in the DVD Example he is identifying the locker using an instance of the fasade (that's why he is sending "this" with the record number to the reservation manager instance).

You can use the same example in the DVD example. But make sure that your interface doesn't require you to return a magical number or cookie.

now you can improve on the design of the locking that you have in the book and most probably you have to make changes to suit your design and data.

hope that helped.

[ November 15, 2007: Message edited by: Musab Al-Rawi ]

Hi Musab,
is it ok to use the methods in Data class like this in update and delete methods handling record locking on specific records wtih recNo

 
            //else update record with new data
           //use reservationmanager with locking unlocking record using recNo
            if( !this.isLocked(recNo) ){
                this.lock(recNo);
                database.updateRecord(recNo, data);
                this.unlock(recNo);
            }

[Conny - I have edited your post to put code between [code] and [/code] UBB tags. This helps keep the code readable. If you would like to see what I have done, please click on the icon that appears at the start of your post. Regards, Andrew]
[ November 22, 2007: Message edited by: Andrew Monkhouse ]

[Musab Al-Rawi]

Sorry for the late reply,
I am not sure if I got your question right or not.

What I understood from my assignment is isLocked() should return true if the record has been locked by the invoking thread, if this method returns false then you shouldn't allow the thread to do update/delete kind of operations.

Another thing to mention is that when you want to lock a record all what you need to do is call the lock() if the record is locked by another thread (user) then your thread should wait until the locking thread unlock() the record.

You don't have to lock on a specific record (as it may be more complicated to implement) UNLESS it is required , i.e say thread 1 is locking record 1 then thread 2 should be able to lock and change recrod 2 if it is not locked. an easier approach is: if thread 1 is locking record 1 then thread 2 can't lock record 2 until thread 1 unlocks record 1.

final thing to mention is always try to follow the following style when locking:

lock(recNo);
try{
    doOperation();
}finally{
unlock(recNo);
}

to make sure that the record is unlocked whether the operation fails or succeeds.

[conny pemfors]

Originally posted by Musab Al-Rawi:
Sorry for the late reply,
I am not sure if I got your question right or not.

What I understood from my assignment is isLocked() should return true if the record has been locked by the invoking thread, if this method returns false then you shouldn't allow the thread to do update/delete kind of operations.

Another thing to mention is that when you want to lock a record all what you need to do is call the lock() if the record is locked by another thread (user) then your thread should wait until the locking thread unlock() the record.

You don't have to lock on a specific record (as it may be more complicated to implement) UNLESS it is required , i.e say thread 1 is locking record 1 then thread 2 should be able to lock and change recrod 2 if it is not locked. an easier approach is: if thread 1 is locking record 1 then thread 2 can't lock record 2 until thread 1 unlocks record 1.

final thing to mention is always try to follow the following style when locking:

lock(recNo);
try{
    doOperation();
}finally{
unlock(recNo);
}

to make sure that the record is unlocked whether the operation fails or succeeds.

Hi Musab.
that part I get...so Iam quit done with that, but, do you use any kind of these operation from the client, or you only do the locking on the db side.

[Alex Belisle Turcot]

Hi,

I think some people include some locking logic on client side...

I only lock/unlock on the server side, the client has no idea he even has to lock/unlock.

When he wants to book, the client calls the "Book" method which will do the "lock, update, unlock" thing.

Regards,
Alex

[Musab Al-Rawi]

Alex's way is better but in order for you to do it you have to provide a higher layer which is the business layer.
The business layer provide the main functionalities from the client point of view, so the client won't have to invoke lock, update then unlock to do the booking operation, instead he will invoke book() which does the locking updating then unlocking.

[conny pemfors]

Originally posted by Musab Al-Rawi:
Alex's way is better but in order for you to do it you have to provide a higher layer which is the business layer.
The business layer provide the main functionalities from the client point of view, so the client won't have to invoke lock, update then unlock to do the booking operation, instead he will invoke book() which does the locking updating then unlocking.

What I have done in the client, is just as you say, I book a record, sends the command to the server, and use the update method from the data class, where I use the lock methods, so I do not involve any threads on the client, but if that is correct I think that is the best solution.

/Conny

[R van Vliet]

Conny,

I agree, I think that is correct and the most elegant solution.

[R van Vliet]

By the way, for those that delegated locking to a seperate class (say LockManager), does anyone guard against things such as :

lockManager.lockRecord(1);
lockManager.lockRecord(1);

Since most operations requiring record lock are atomic and are guaranteed to unlock the record before returning this isn't an implementation problem, but looking solely at the scope of LockManager wouldnt it be better to disallow a thread locking the same record twice? I havent noticed anyone doing or discussing this, hence asking.

[conny pemfors]

Originally posted by R van Vliet:
By the way, for those that delegated locking to a seperate class (say LockManager), does anyone guard against things such as :

lockManager.lockRecord(1);
lockManager.lockRecord(1);

Since most operations requiring record lock are atomic and are guaranteed to unlock the record before returning this isn't an implementation problem, but looking solely at the scope of LockManager wouldnt it be better to disallow a thread locking the same record twice? I havent noticed anyone doing or discussing this, hence asking.

what I believe or think in my mind, is that you always should check if the record is already locked, in that case, that kind of scenario would not be any problem, can't lock a locked record.

when you do the lock operation, check first it is not already locked.
so if you have many clients connected, it always checks before whether the recordNo is locked by another user.


Conny

[Alex Belisle Turcot]

I think you raised something...

In my Data... if the same thread lock a record and then tries to re-lock it... it will wait. It will wait until another request comes in...

I think the lock method should detect that it's the same Thread and allow him to go through... cause heh, he got the lock already!

Thanks for your tip!
[ November 19, 2007: Message edited by: Alex Belisle Turcot ]

[R van Vliet]

Originally posted by conny pemfors:


what I believe or think in my mind, is that you always should check if the record is already locked, in that case, that kind of scenario would not be any problem, can't lock a locked record.

when you do the lock operation, check first it is not already locked.
so if you have many clients connected, it always checks before whether the recordNo is locked by another user.


Conny

Hi Conny,

Ofcourse the code checks if the record is already locked, that's the purpose of having a lock. But there's a significant difference between the same thread trying to acquire a lock twice, or two different threads.

In almost all lock code I've seen, the code checks if that record has been locked, and if so puts the thread in getState() == State.WAITING and releases cpu. This is intended behaviour when two seperate threads attempt to acquire a lock.

However, if you look at my little code sample, the same thread attempts to lock a record twice. If you then follow the "normal" locking approach, the following happens :

T1 : lockRecord(1);
T1 : record not locked, so acquire lock and return cookie
T1 : lockRecord(1);
T1 : record locked, so let T1 wait for unlock of record 1

The problem here ofcourse is that T1 is the thread that's supposed to do the unlocking, which it cant because it's not in WAITING (ofcourse it's theoretically possible for another thread to unlock the record, but that requires the cookie and I cant think of a practical solution where one thread does the locking and another does the unlocking).

The only safeguard against this is keeping track of which thread owns a lock along with which client (represented in my assignment by a cookie). Then if one thread tries to lock a record twice the second time will simply be ignored (meaning it will return the same cookie as the first attempt and not block).

I think this would be best implemented by having some value object LockRecord that has a recNo, cookie and thread values and manage those accordingly.

That aside, I think another discussion is whether or not you should guard against it in the first place. If you define a LockManager.lockRecord contract that specifies a thread should only call the method once per recNo arent you in the clear already? Most Data implementations or it's delegate classes usually have atomic operations, so there's a guarantee that lock and unlock are called within such an atomic block.

In other words, should we make LockManager safe as a seperate component, or take into consideration it's a private delegate class of a component that will not break the above contract anyway.

Opinions?
[ November 19, 2007: Message edited by: R van Vliet ]

[Andrew Monkhouse]

Hi,

Originally posted by R van Vliet:
[...] if you look at my little code sample, the same thread attempts to lock a record twice. If you then follow the "normal" locking approach, the following happens :

T1 : lockRecord(1);
T1 : record not locked, so acquire lock and return cookie
T1 : lockRecord(1);
T1 : record locked, so let T1 wait for unlock of record 1

The problem here ofcourse is that T1 is the thread that's supposed to do the unlocking, which it cant because it's not in WAITING[...]

I personally think you need to be aware of what is stated in your instructions, as well as what is reasonable - the 2 may conflict.

There are multiple versions of this assignment, with multiple lock method signatures and comments. To give 2 examples:

Locks a record so that it can only be updated or deleted by this client.

If the specified record is already locked by a different client, the current thread gives up the CPU and consumes no CPU cycles until the record is unlocked.

Locks a record so that it can only be updated or deleted by this client. If the specified record is already locked, the current thread gives up the CPU and consumes no CPU cycles until the record is unlocked.

The 1st example above makes it clear (at least to me), that in the cases where a client attempts to lock a record twice then it is acceptable for the second attempt to succeed.

The 2nd example above seems more dogmatic - if the record is locked [irrespective of who locked it] then all attempts to lock the record should result in the Thread waiting - effectively a deadlock.

Unfortunately there are some pedantic assessors out there, and some of them have failed candidates for deviating from the instructions even when the deviation makes perfect sense and was well documented in code and in documentation (these candidates have all passed on appeal though).

Originally posted by R van Vliet:The only safeguard against this is keeping track of which thread owns a lock along with which client (represented in my assignment by a cookie). Then if one thread tries to lock a record twice the second time will simply be ignored (meaning it will return the same cookie as the first attempt and not block).

Remember if you are using RMI and you are calling the lock() update() and unlock() methods all through RMI then you will not be able to use the thread to uniquely identify the client. (Not everyone (or even the majority) agrees with me that you should call all 3 methods from the client though ).

Regards, Andrew

[R van Vliet]

Hi Andrew,

Thanks for your reply and I agree, although I think in the case of the 2nd instructions the safer bet is to let the code follow the instructions to the letter (avoiding the woes of assessors that got a speeding ticket that morning) and document your reservations in choices.txt.

You make a good point about RMI. Although perhaps even in the case of RMI tracking the thread would be a viable option. I wouldnt use the thread to uniquely identify a client to begin with, but it's still a way to avoid the deadlock situation mentioned. All you'd basically care about is one thread trying to lock record X twice. After that check you can always seperately check if you're talking to the same client and deal with the request to lock appropriately, still avoiding the deadlock in the process.

I almost feel a need to apologise for the fact that I too belong to that majority that objects calling lock/unlock from the client

[conny pemfors]

Originally posted by R van Vliet:
Hi Andrew,

Thanks for your reply and I agree, although I think in the case of the 2nd instructions the safer bet is to let the code follow the instructions to the letter (avoiding the woes of assessors that got a speeding ticket that morning) and document your reservations in choices.txt.

You make a good point about RMI. Although perhaps even in the case of RMI tracking the thread would be a viable option. I wouldnt use the thread to uniquely identify a client to begin with, but it's still a way to avoid the deadlock situation mentioned. All you'd basically care about is one thread trying to lock record X twice. After that check you can always seperately check if you're talking to the same client and deal with the request to lock appropriately, still avoiding the deadlock in the process.

I almost feel a need to apologise for the fact that I too belong to that majority that objects calling lock/unlock from the client

Hi Everybody!
Anybody who know for sure how well documented choices.txt should be.
Somehow it must be explained in detail of why certain choices have been made regarding record locking for instance, or other issues.


/Conny

[Alex Belisle Turcot]

Hi,

I'm not there yet.. in 2 or 3 weeks I believe..

This has been ask many times and there are few very good thread where people detail what they did...
I read some say it should not be much more than 1 page. Maybe 2-3 top.
Look it up yourself on the board, you'll find a lot of people shared their experience about their choices.txt

Regards,
Alex
[ November 20, 2007: Message edited by: Alex Belisle Turcot ]

[Andrew Monkhouse]

Originally posted by R van Vliet:
I almost feel a need to apologise for the fact that I too belong to that majority that objects calling lock/unlock from the client

Heh - no problems. For what it's worth, I also object to calling lock/unlock from the client. However I believe that this is what is required by the instructions.

Originally posted by R van Vliet:
You make a good point about RMI. Although perhaps even in the case of RMI tracking the thread would be a viable option. I wouldnt use the thread to uniquely identify a client to begin with, but it's still a way to avoid the deadlock situation mentioned. All you'd basically care about is one thread trying to lock record X twice. After that check you can always seperately check if you're talking to the same client and deal with the request to lock appropriately, still avoiding the deadlock in the process.

I deliberately commented on your responses out of order since your comment on using threads as identifiers in RMI only makes sense in the case where you have the entire booking method server side. In that case you probably don't need any other checks - the thread should be able to indicate ownership. However if you have the ability to call lock from client side, then you cannot use the thread as an identifier - thread reuse in RMI could result in different clients using the same thread.

Regards, Andrew

[Andrew Monkhouse]

Hi,

Originally posted by conny pemfors:
Anybody who know for sure how well documented choices.txt should be.
Somehow it must be explained in detail of why certain choices have been made regarding record locking for instance, or other issues.

We generally recommend using one thread per question - you may miss valuable replies by adding new questions to this existing topic.

I agree with Alex - 1 page is ideal, 2 or 3 pages tops.

You do not have to write essay style - it is quite acceptable to write bullet points. This can be very beneficial if you are writing the choices.txt document in some language other than your primary language. It is also good for when you do the part II exam - you can usually just provide the corresponding bullet points in the exam.

Having said that, I failed miserably at following my own advice. I remember at one point complaining that my document was at 6 pages and growing. I think it was around that time that someone suggested I could be writing a book.

Regards, Andrew

[R van Vliet]

Originally posted by Andrew Monkhouse:
Heh - no problems. For what it's worth, I also object to calling lock/unlock from the client. However I believe that this is what is required by the instructions.

Could you elaborate on this? Do you mean the instructions imply the data access interface (DBAccess in my case) should be the interface that is optionally remote? And do you feel this way regarding all current assignments or just the (i assume significantly older) one you implemented? I've always assumed the DBAccess interface simply defined an interface for the data layer rather than the business layer.

Originally posted by Andrew Monkhouse:
I deliberately commented on your responses out of order since your comment on using threads as identifiers in RMI only makes sense in the case where you have the entire booking method server side. In that case you probably don't need any other checks - the thread should be able to indicate ownership. However if you have the ability to call lock from client side, then you cannot use the thread as an identifier - thread reuse in RMI could result in different clients using the same thread.

Regards, Andrew

I suppose this is what happens when I try to explain a thought in a language that isn't my native one

What I was trying to get across is not mutually exclusive with your point above (i think?). I meant that in RMI regardless of using the thread as a client identifier or not, you'd still need to keep track of which thread attempts to lock which record. As you said threads could be reused, so two seperate lock() calls from two seperate clients attempting to lock the same record could perhaps be executed by the same thread, still causing a "deadlock" (in quotes here because if my understanding of RMI is correct it's likely the following unlock calls are executed by another RMI thread, releasing the thread currently waiting for the record lock).

Perhaps there are just some gaps in my understanding of RMI, i rarely use it, feel perfectly free to correct any incorrect assumptions on my part and thanks for the feedback

P.S. I'm assuming RMI works with a thread pool of some sort, so that two seperate remote method calls could be executed by the same RMI thread. I couldnt google anything useful on this on short notice, but if this isn't so perhaps my nasty scenario cant even happen.
[ November 20, 2007: Message edited by: R van Vliet ]

[Andrew Monkhouse]

Originally posted by Andrew Monkhouse:
Heh - no problems. For what it's worth, I also object to calling lock/unlock from the client. However I believe that this is what is required by the instructions.

Originally posted by R van Vliet:
Could you elaborate on this? Do you mean the instructions imply the data access interface (DBAccess in my case) should be the interface that is optionally remote? And do you feel this way regarding all current assignments or just the (i assume significantly older) one you implemented? I've always assumed the DBAccess interface simply defined an interface for the data layer rather than the business layer.

Yes - I feel that the interface you have been asked to implement should be accessible client side. This only applies to the newer assignments - the older (Fly By Night) assignments did not have an interface to implement. As for my logic as to why I believe this, take a look at this topic. It is rather lengthy, and I expanded on my ideas as I went, but I think I make a valid case for the instructions requiring the interface to be exposed to the client.

However, as that thread shows, there are opposing arguments. And it certainly appears that assessors are not failing people for only exposing business methods over the network. So the argument is kind of academic.

Originally posted by R van Vliet:
P.S. I'm assuming RMI works with a thread pool of some sort, so that two seperate remote method calls could be executed by the same RMI thread. I couldnt google anything useful on this on short notice, but if this isn't so perhaps my nasty scenario cant even happen.

The RMI specification is quite explicit in saying that it refuses to define what anything to do with how threading is handled on the server. So your nasty scenario is allowed according to the specification.

I have tested this on multiple versions of the Sun JDK up to version 1.5, and I have been able to confirm the scenario you mention. I also just tested it with the Apple JDK, and with 2 clients connecting to the server 12 times each, I found that for my particular code (which I will admit is designed to prove this problem ) there were only 3 threads used. One thread got used 8 times by each of the 2 clients, another thread got used 3 times by each of the 2 clients, and another thread got used once by each of the 2 clients. If you are really interested in the proof, let me know and I will post the code, however at 67 lines long it may be a bit of overkill to post the code just to prove a problem that wont affect anyone only exposing business objects to the client.

Regards, Andrew

[R van Vliet]

Originally posted by Andrew Monkhouse:
However, as that thread shows, there are opposing arguments. And it certainly appears that assessors are not failing people for only exposing business methods over the network. So the argument is kind of academic.

Agreed, but then academic discussions are good brain food nonetheless

Originally posted by Andrew Monkhouse:
The RMI specification is quite explicit in saying that it refuses to define what anything to do with how threading is handled on the server. So your nasty scenario is allowed according to the specification.

I have tested this on multiple versions of the Sun JDK up to version 1.5, and I have been able to confirm the scenario you mention. I also just tested it with the Apple JDK, and with 2 clients connecting to the server 12 times each, I found that for my particular code (which I will admit is designed to prove this problem ) there were only 3 threads used. One thread got used 8 times by each of the 2 clients, another thread got used 3 times by each of the 2 clients, and another thread got used once by each of the 2 clients. If you are really interested in the proof, let me know and I will post the code, however at 67 lines long it may be a bit of overkill to post the code just to prove a problem that wont affect anyone only exposing business objects to the client.

Regards, Andrew

Hehe, thank you for putting time into confirming my theory. And I'll take your word for the fact that your proof is sound.

So would you say this means that all assignments that use RMI and expose the lock methods to the clients without guarding against this specific scenario are broken? I dont have the code included with the book at hand, but does it guard against this issue? I dont recall seeing it in there but I must admit I only quickly browsed through it (the code that is) and that was a fair few months ago.

[conny pemfors]

Originally posted by Alex Belisle Turcot:
Hi,

I'm not there yet.. in 2 or 3 weeks I believe..

This has been ask many times and there are few very good thread where people detail what they did...
I read some say it should not be much more than 1 page. Maybe 2-3 top.
Look it up yourself on the board, you'll find a lot of people shared their experience about their choices.txt

Regards,
Alex

[ November 20, 2007: Message edited by: Alex Belisle Turcot ]

Hi Alex, Iam getting quite confused now.
When it first comes to CPU cycles, I thought at first that using the lock would be enough in update and delete methods, but then you are thinking about threads, where do you use threads on the db side.
for instance...

 /**
    * 
    * Class used with LockThread on delete operation
    * Takes data class and recNo as parameters 
    */ 
    class LockRunnable implements Runnable{
        Data data;
        int recNo;
       
        public LockRunnable(){}
        public LockRunnable(Data data, int recNo){
            this.data = data;
            this.recNo = recNo;
           
        }
 
        /***** synchronize this class and with reservations manager.****/
        public void run(){
            try{
                synchronized(data){
                    while( reservationsManager.isLockingRecord(recNo) ){
                        reservationsManager.wait();
                    }
                    //do some operation
                    reservationsManager.notifyAll();
                }
            }catch(InterruptedException ie){
                
                ie.printStackTrace();
                
            }
        }
    }

[Conny - I have edited your post to put code between [code] and [/code] UBB tags. This helps keep the code readable. If you would like to see what I have done, please click on the icon that appears at the start of your post. Regards, Andrew]
[ November 22, 2007: Message edited by: Andrew Monkhouse ]

[R van Vliet]

Hi conny,

I was unable to extract a question from your post. Could you please be more specific as to what you are confused about?

Also, on a sidenote, it's much easier to read code if you use the appropriate code tag.

[conny pemfors]

Originally posted by R van Vliet:
Hi conny,

I was unable to extract a question from your post. Could you please be more specific as to what you are confused about?

Also, on a sidenote, it's much easier to read code if you use the appropriate code tag.

it's the part where you use threads,
I might have another solution that looks like this, but I don't kn ow if it is correct but it seems to work.
used during update a record.
Now I use Reentrant lock as well in reservationsmanager.

 synchronized(reservationsManager){
                    
               
                        while( isLocked(recNo) ){
                      
                                reservationsManager.wait();
                      
                        }
                    
                    
                    this.lock(recNo);
                    //throws a modification exception if something has happened to the record data after calling the update method
                    String[] tmpData =read(recNo);
                    Record tmpRecord = new Record();
                    tmpRecord.setId(recNo);
                    tmpRecord.setData(tmpData);
                    
                    if( !tmpRecord.equals(record)){
                            throw new ModificationException("Modification Failure record " + record.getId());
                    }else{
                        log.info("record has not been updated since locking of recNo" + recNo );
                    }
                    database.updateRecord(recNo, data);
                    
                    
                    
                    
                    reservationsManager.notifyAll();
                    
               }//end synchronized

[Conny - I have edited your post to put code between [code] and [/code] UBB tags. This helps keep the code readable. If you would like to see what I have done, please click on the icon that appears at the start of your post. Regards, Andrew]
[ November 22, 2007: Message edited by: Andrew Monkhouse ]

[Andrew Monkhouse]

Hi

Originally posted by R van Vliet:
So would you say this means that all assignments that use RMI and expose the lock methods to the clients without guarding against this specific scenario are broken?

I would say that any assignments that expose the lock method to clients and use the RMI thread ID to identify the client on the server are potentially broken.

Having said that, there are assignments that may not need to identify the client - as we originally discussed there are instructions that seem to indicate that a recursive lock should block, so identifying the client gains you nothing for that.

Originally posted by R van Vliet:
I dont have the code included with the book at hand, but does it guard against this issue?

I don't have time to look into Terry's code at the moment (he wrote the Sockets and RMI chapters), but from memory we guard against this.

Regards, Andrew

[R van Vliet]

I wrapped your code samples in the appropriate tags in my reply. In the interest of our sanity please do the same in the future , it makes it much easier to read. I tried to put your code blocks inside code tags in your posts but (sensibly) the JavaRanch people in power dont allow me to do so.

Originally posted by conny pemfors:


it's the part where you use threads,
I might have another solution that looks like this, but I don't kn ow if it is correct but it seems to work.
used during update a record.
Now I use Reentrant lock as well in reservationsmanager.

synchronized(reservationsManager){
                                   
    while( isLocked(recNo) ){
                      
        reservationsManager.wait();
    }                 
}
[CODE]
[/quote]
      
This, in and by itself, looks fine. I'm assuming it's part of your lock(Record) method.             
 
 [quote]Originally posted by conny pemfors:
                    
[code]
                    this.lock(recNo);
                    //throws a modification exception if something has happened to the record data after calling the update method
                    String[] tmpData =read(recNo);
                    Record tmpRecord = new Record();
                    tmpRecord.setId(recNo);
                    tmpRecord.setData(tmpData);
                    
                    if( !tmpRecord.equals(record)){
                            throw new ModificationException("Modification Failure record " + record.getId());
                    }else{
                        log.info("record has not been updated since locking of recNo" + recNo );
                    }
                    database.updateRecord(recNo, data);
                    
                    
                    
                    
                    reservationsManager.notifyAll();
                    
               }//end synchronized
[code]
[/quote]
 
Here I have a few questions. First of all I dont see an unlock call anywhere but perhaps that got lost in the copy/paste (although this should happen before the notifyAll). Also, does tmpRecord.setData actually persist the record to the database file, because if not isn't it extremely unlikely that this if statement evaluates to true? :
 
[code]
    if( !tmpRecord.equals(record)){

I suppose it depends on the implemention of equals(), but if it checks if recNo and the data strings are equal and setData does nothing more than this.data = data I dont think this can actually happen.

The else clause then has the exact opposite problem. Provided your record locking system works it cannot happen that your record is being modified after the record was locked, so checking for this is a bit redundant. Ofcourse, ignore that comment if the else-log line is for debugging purposes only or if you allow other parts of the application to directly modify your value object (Record) without checking for a lock on that record. The latter would be something I'd avoid, the Record set..() methods should have package visibility only really.

Anyway, I hope this clears one or two things up. If you have a more specific question feel free to post it and I'll try to be more specific if I can

[R van Vliet]

Originally posted by Andrew Monkhouse:

I don't have time to look into Terry's code at the moment (he wrote the Sockets and RMI chapters), but from memory we guard against this.

I assumed so, thanks for your input on the discussion

[MarioAixel Rodriguez Jaen]

You guys should check the Lock interface in Java 6. It provides with a very useful object, ReentrantLock, that fixes all those problems:
1.- how to figure out if it is the same thread that is requesting a lock,
2.- how to set timeouts, and
3.- how to acquire a lock until the same thread releases it.

So next time you'll tell your girlfriend that you have to stay coding all weekend long, be careful. She could say, check out:

Check this out
http://java.sun.com/javase/6/docs/api/java/util/concurrent/locks/ReentrantLock.html"

and you could be forced to watch Titanic again =)
[ November 22, 2007: Message edited by: MarioAixel Rodriguez Jaen ]

[R van Vliet]

Hi Mario,

Quite a lot of people use Lock (ReentrantLock specifically in this case) to implement their locking strategy. Unfortunately it is not a miracle cure for the problems specified above. In it's basic usage pattern :

lock.lock();
 
try {
    // do your magic
} finally {
    lock.unlock();
}

is not different to (here "lock" is just an Object for example) :

 
synchronized(lock) {
    // do your magic
}

The java.util.concurrency package does start to shine when you want to do more complicated things related to locking and concurrency. In most cases I tend to lean towards synchronized blocks rather than using a Lock unless it offers me specific advantages in that particular piece of code.

That said, it is a good suggestion to check them out, ReadWriteLock for example can certainly be useful for SCJD assignments.

[Andrew Monkhouse]

Originally posted by MarioAixel Rodriguez Jaen:
You guys should check the Lock interface in Java 6. It provides with a very useful object, ReentrantLock, that fixes all those problems [...]

Unfortunately it does not handle the case where a single client calls the (reentrant) lock twice, but happens to use a different thread for the second call. This is very plausible with RMI.

It might also cause you problems if your instructions state that the second call to lock() should block, even when called by the same client in the same thread.

Originally posted by MarioAixel Rodriguez Jaen:
and you could be forced to watch Titanic again =)

Regards, Andrew