My Locking Approach

[Alan Morgan]

Hey all,

I'm farily close to submitting (well hopefully anyway) and I was hoping that someone may have a quick look at my locking approach before I do as I really want to get this part yet.
At textual description of it is below.
If I am leaving out critical info please let me know

Thanks a lot,
Alan.


My Data class is a singleton and is therefore shared between multiple
clients. This class contains a single LockManager class to which it
delegates the locking and unlocking of records.

The LockManager contains a HashMap which maps record numbers to lock
cookies.
If a record number is not in the HashMap then it is not locked.
When you lock a record you are provided with a unique lock cookie.
When you wish to perform operations that require this lock cookie then the LockManager checks that the record you are trying to change matches the lock cookie that you provide.
When a record is unlocked then the entry in the HashMap for that record is removed.

The methods from Data that require a record to be locked are update(..) and
delete(..).

The contract that the client must follow in calling the server is as follows

* call lock on data layer
* run method that requires lock
* call unlock on data layer

Notice that since the delete method is not required on the GUI then the only method that follows this contract is ControllerImpl.bookRecord(..) as it requires an update(..) to be called on the data layer.

This contract is enforced to a certain degree by the LockManager.
If the client tried to call update without first having locked the record then it would be prevented from doing so by the LockManager.
However if the client locked the record but then did not unlock it wouldremain locked. A lock timeout policy was one possible way around this but I believe that to be outside the scope of this assignment.

LockManager methods:
lock - First synchronizes on the lockMap
Then a while loop is created to check if the record already exists in
the lockMap. If it is then the record is already locked so we call wait on the lockMap which results in us giving up the CPU as per the
requirments.
When the thread is again in the running state it re-checks if the
record is locked.
It also re-checks that the record is still valid. This is important as it could have been deleted while we were waiting on it.
If it is not locked then we generate a lockCookie and place this in the
HashMap with the record number as key and return the cookie to the caller.

unlock - First synchronizes on the lockMap.
Then we check if the record is locked. If it is not we return as we
cannot unlock a record that is not locked.
If it is locked then we check that the cookie passed in is the one corresponding to the record number in the lockMap.
If not we throw a SecurityException.
If it is then we remove the entry and call notifyAll() to signal to that any threads that waited in the lock method on this record that it is now unlocked

validateLock
This is a helper method.
First synchronizes on the lockMap.
Then we check if the passed record number is locked with the passed lock cookie
If not we throw a SecurityException

[Yupp Cook]

Hi Alan

I indepentently and surprisingly came to almost the same locking schema. To see yours beeing that similar relieves me of my doubts.
Here are some considerations that may be useful:

a)
Locking: Your wrote >>>...When the thread is again in the running state it re-checks if the record is locked.
It also re-checks that the record is still valid. This is important as...<<<
What do you mean by "re-checks if record is still valid"? Re-checking means checking again? Where it checks the validity in the first place? I think just checking when back in running state is enough.

b) Why don't you synchronize on the LockManager instead of lockMap? It's just a small difference and doesnt change locking mechanism but you wouldn't need to get lockMap from the LockManager.

c)In my scenario an SecurityException is thrown even on trying to unlock an not-locked record. You just return not letting the caller know. I m not sure which way is better. But sth. on the client must have gone wrong if this happens. So I feel better throwing the excpt.

d) You are using the validatLock method inside update? If so, wouldn't it be better to synchronize inside the update method? Just synchronizing inside validateLock still allows acces by 2 different updates. That's to debate... I'm not sure about it.

e) How s your update method. Do you first check if the record is valid and then read or viceversa? Another point I'm not sure which is better.

What's the advantage of having Data as a singleton?

Saludos
Yupp

[Yupp Cook]

Sth. else came to my mind. What if an Exception occurs while unlocking. Do records remain locked then? Is the Client handling this?

[Alan Morgan]

Hi Yupp,

Thanks for the feedback, good to know someone is thinking along the same lines as me. To answer your questions:

a) I check the validity at the top of the while loop
Re-check means I check again
In other words I want to make sure it hasn't been deleted while the thread was waiting.

b) I don't need to get the lockMap from the LockManager as the methods in question are in the LockManager so the lockMap is accessible to me.

c) Well spotted - I changed my method to throw a SecurityException

d) The validateLock method is used by both the update and delete
It is synched as is the access to the Data file in both the update and delete. Does this make sense ?

e) I read first. Can't think of a reason not to do it this way.

Advantange of Data as singleton ?
You know what I can't seem to remember why :roll:
I guess it was either that or make my LockManager a singleton
Or have a static lockMap on LockManager
I'm sure I must have it written down somewhere whi I did it.
(It was a long time ago when I made that decision )




Erwin,

My create method is fully synchronized so as to avoid that.

[Yupp Cook]

Hi Alan

It took a while but I found the time to go on with it.

a) The 2cnd check is necessary. I don't understand the 1st: If it's valid: fine - you enter the while loop. If it's invalid: It can only be locked for the very unlikely case that it just has been deleted then it waits for the lock otherwise it gets it immediatly. Then it checks the validity. So the 1st check is unnecessary but doesn't harm.

b) Ok. I synch on the LockManager itself.

c)

d) I can't really tell without seeing the details.

e) You are right, I was just confused ;-)



How long were you working on it?
If you want describe your synchronization approach, too. And I ll have a comparison look, must be similar. I have a special question already: Do you synch find/delete/create/update on a common object?

[Alan Morgan]

Originally posted by Yupp Cook:

How long were you working on it?
If you want describe your synchronization approach, too. And I ll have a comparison look, must be similar. I have a special question already: Do you synch find/delete/create/update on a common object?

I've been working on it since last March...I took 3-4 months off in between though.

Yes I synch all those methods on a common object....the RandomAccessFile (RAF) which I use to access my data file.

My sychronization approach is basically that I synch all access to the RAF and all access to my lockMap.
I also synch two methods:
1) My Data constructor so as to make it a thread safe singleton
2) create method as my requirements say to use a deleted record when creating so I synch it so that no record is deleted mid-create which would result in me creating my record in the wrong place.

Is this similar to yours ?

[Yupp Cook]

Hi

Pretty much the same even though my Data isn't a singleton.
I also synch all access to lockMap and raf. Method create is synched in itsself.

I'm still debating to add 3 more synch objects:
- on find/delete to prevent that a record dissappears while find runs
- on find/update to prevent that a record is changed while find runs
- on find/create to prevent that a record is created while find runs(this also would prevent create from interacting with another create)

Yupp

[Robert Alden]

Alan

Is your assignment not B&S? (Since delete method is not required on GUI)

Do you need to validate the data in GUI cache against the data in file (besides making sure the record is not deleted), before updating the record ?
In other words, should the client know what data it is overwriting ?


Your mail was helpful.

Best wishes
-Rob

[Alan Morgan]

Yupp,

I can see why you might want to synch on your find operations but I had this thought and Andrew explained that I can never be sure that my data is not out of synch.

Imagine the scenario where you do a find on all records.
You synch so that you're sure the data returned is correct.
So record 2 is not booked.

Now a second after your find is complete someone books record 1. Now the data on your GUI is out of date.
So what did the synch gain you ? 1 sec of correctness (or 1 millisecond potentially)

The only way to be sure that the data your client sees is always up to date is to have your server push all changes to the client.
But this is going to create lots of network traffic and is not asked for in requirements so I reckon don't do it.

So basically I accepted thatr my data could be out of date and didn't do the extra synchs you mention above.

I hope that makes sense.



Robert,

My assignment is URLyBird and the delete method is not required by the GUI but I implement it as it must be implemented in Data to fulfill the DB interface.

Do you need to validate the data in GUI cache against the data in file (besides making sure the record is not deleted), before updating the record ?
In other words, should the client know what data it is overwriting ?

I don't keep a cache per se...I immediately write all operations to data file and read directly from it.
But if you are talking about making sure that my Data is up to date then see my reply to Yupp above.

Does that answer the question ?

[Yupp Cook]

Makes a lot of sense and keeps synchronization quite simple. Very good!

When you read/write always immediatly from/to the file how do you handle the IOExceptions that arrive in Data? Changing them into RuntimeExceptions?

[Alan Morgan]

Originally posted by Yupp Cook:
Makes a lot of sense and keeps synchronization quite simple. Very good!

When you read/write always immediatly from/to the file how do you handle the IOExceptions that arrive in Data? Changing them into RuntimeExceptions?

Glad to hear some words of encouragement Yupp, makes me a little bit more confident about what I'm doing

Yes I change them into RuntimeExceptions.
I create a DataException which extends RuntimeException and throw this.

If the error occurs in the Network layer I throw a RemoteDataException which also extends RuntimeException.
I made this distinction just to make it obvious where the error is occuring.

I take it by the way you asked the question that you also change them to RuntimeExceptions ?

[Yupp Cook]

No reason to have doubts with your solution, seems very sound.

Indeed I change some checked exceptions like IOException and UnsupportedEncodingException into subclassed RuntimeExceptions just to fit into the given Data schema. I feel weird doing that though.
Maybe I ll change it, but this would mean to keep a runtime copy of the records = big effort

[Alan Morgan]

Originally posted by Yupp Cook:
No reason to have doubts with your solution, seems very sound.

Indeed I change some checked exceptions like IOException and UnsupportedEncodingException into subclassed RuntimeExceptions just to fit into the given Data schema. I feel weird doing that though.
Maybe I ll change it, but this would mean to keep a runtime copy of the records = big effort

The idea of subclassing RuntimeException has been well discussed on this forum and seems to have been agreed upon as the preferred approach in this situation so I'm happy to stick with it.

I'm guessing you'll also be happy to do this to avoid the big effort

[Yupp Cook]

So, you are about to submit? Good luck!

I ve finished the backend part. Now about to begin with the network stuff. I don't really know how and where to start yet. You got any recommendations?

[Alan Morgan]

Having someone test it now and tying up bits...hopefully submit within the next two weeks.

Recommendations for Network layer..mmm...
Think about using the Adapter pattern
You also want to make sure that no matter whether you are local or remote that your client/controller is dealing with the same interface.
Decide early whether you want a fat or thin client i.e. controller on client or server side (I chose fat but if I had it to do over again I might choose thin)
Have a look at what Andrew's book has to say on it ?..I haven't read it but if his contribution on here is anything to go by he won't send you far wrong.

Any other questions let me know

Alan

[Alan Morgan]

Hey guys,

I'm within days of finishing this project now and I was just reading back over my locking and threading approach in the thread above and it got me thinking about deadlock.

And then I got scared...I haven't really consciously programmed for deadlock avoidance.
I made a conscious decision to not deal with orphaned locks as I believe it to be outside the scope of the assignment.

I took a piece of advice from another thread and ran a test whereby I created 50 threads and had them update a single record 100 times each.
The results should have been that the fields value was 50000.
This proved to be the result.

In a previous thread on this subject Andrew said the following:

Originally posted by Andrew Monkhouse:
Deadlock handling does not need to be convoluted - the logic for "one client can only lock one record at a time" can be done simply, as can "clients can only lock multiple records if they lock them in numerical order". Allowing infinite locks to be granted in any order to any client can require more complex deadlock detection, but it can still be done cleanly and elegantly.

Do I take this to mean that you should hold a map of clients to records locked and before allowing a record to be locked by a client make sure that he has not locked another one ?

Any thoughts would be greatly appreciated.

[Yupp Cook]

Hi Alan

Still around here?

My way is it to block the client who sent a request from any further action til he got the response. This implies that each client always runs in just one thread never starting a second.

The server follows the order:
receiveRequest - lock - dbAccess - unlock - returnResponse



Regards
Yupp

[Alan Morgan]

Hi Yupp,

Ya still here :roll:


So what is to stop a client from selecting record 1 to book and then selecting record 2 to book before the response has been received ?

Does the server hold onto the client id or something and not allow him to lock any records until the response is received.

[Yupp Cook]

No, the client blocks himself til he gets the response, so no second request can be sent.
I m not sure if this is the best way to do it, but it keeps the client away from booking multiple records.

Y.

[Alan Morgan]

Mmmm....I would have thought that that kind of functionality would be best placed on the server side.

What are your thoughts on the approach I mentioned above as outlined by Andrew:

Lock
Put record number and lockCookie into LockMap
Also put client id into ClientMap

Scenario: Client A attempts to book record 1 with cookie 123
LockMap - key is 1 and value as 123.
ClientMap - key is A
Client A attempts to book record 2 with cookie 456 while record 1 still locked

We check first to see if Client id is in clientMap.
If it is we make the thread wait.

Unlock
Remove entry from both maps

Thoughts ?

[Richard Brooks]

Alan,

I'm still in design phase for locking, so I came upon your description while trolling for ideas. Thank you for posting it in such easily digested detail.

I drug out a legal pad and #2 software-design-instrument and drew some boxes and arrows and I think I pretty much follow your design. One question I did have was about this comment in LockManager::lock():

When the thread is again in the running state it re-checks if the
record is locked.
It also re-checks that the record is still valid. This is important as it could have been deleted while we were waiting on it.

I follow your rationale here...I can see it but...it looks to me like this may be a bit of encapsulation leakage, unless I'm interpreting it incorrectly, which may well be the case. What I mean is that this seems to require that the LockManager have information about the concept of "valid" in data records, which seems a bit outside the very sleek and otherwise elegant design you've come up with for it dealing only with record numbers and locking. Which means, in an implementation sense, you have to supply something, not sure what...a data structure maybe...?..from your Data class to LockManager? Or am I just waaay out in the weeds somehere?

Thanks,
Richard

[Alan Morgan]

Originally posted by Richard Brooks:
I follow your rationale here...I can see it but...it looks to me like this may be a bit of encapsulation leakage, unless I'm interpreting it incorrectly, which may well be the case. What I mean is that this seems to require that the LockManager have information about the concept of "valid" in data records, which seems a bit outside the very sleek and otherwise elegant design you've come up with for it dealing only with record numbers and locking. Which means, in an implementation sense, you have to supply something, not sure what...a data structure maybe...?..from your Data class to LockManager? Or am I just waaay out in the weeds somehere?

Ok Richard I see where you are coming from.
The LockManagers job is to lock and unlock the record and not to do anything else.
So checking a records validity is outside the scope of his remit so to speak.
Basically what I do is call isValidRecord(recNo) which is a static method on my Data class.
That way the LockManager doesn't care about how to check its validity he just knows it has to be checked and lets Data worry about doing it.

Would you be happy with that scenario ?

Thanks,
Alan.

[Richard Brooks]

Alan,

Yes, right. That's exactly what I was talking about. Well summarized. Thanks for the clear answer/explanation of what you've done.

Just playing devil's advocate here, understand...umkay?...cause I think thats' what'd be most helpful to both of us..okay?

<devils-advocacy>
Your implementation of how LockManager deals with the concept of record validity minimizes the leakage in terms of lines of code and simplicity of the interface between the Data class and the LockManager, yes. But...it's still there in an OO design sense. So your LockManager implementation is forever bound (by a single teeny-weeny method call, granted) to the Data class/DBAccess interface.

Just off the top of my head, though, I wonder if by letting the LockManager not care about validity of the record but letting your Data class be the only one to know/care about could be made to work? After all, the Data class is very familiar with record validity and knows how to compute it...I wonder if we could help him keep that nasty secret to himself.

That way LockManager, lucky simple-minded young scoundrel he is, could dispense locking/unlocking calls all he wanted with nary a thought about records or valiidyt. Could it be made to be incumbent on the Data class to check...once the lock was obtained...as to whether the record was still valid or not? Once "owned", in a locked sense, then Data could make up it's own mind about what to do if he got an invalid record, like throw a RecordNotFoundException or something after unlocking it....?
</devils-advocacy>

I dunno....it just seems to me with the static reference to Data removed, you'd free up LockManager to be used by other classes outside DBAccess and it'd be one less little arrow I'd need to draw on my legal pad here. That's my biggest problem, no room left on the page.

...but if this makes no sense based on what you're got, ignore me. Remember, I'm still in the almost-no-design, certainly-no-code-blue-sky stage, so everything works correctly, cleanly and perfectly in my B&S system.

Good luck,
Richard

[Alan Morgan]

Originally posted by Richard Brooks:
Alan,

Yes, right. That's exactly what I was talking about. Well summarized. Thanks for the clear answer/explanation of what you've done.

Just playing devil's advocate here, understand...umkay?...cause I think thats' what'd be most helpful to both of us..okay?

<devils-advocacy>
Your implementation of how LockManager deals with the concept of record validity minimizes the leakage in terms of lines of code and simplicity of the interface between the Data class and the LockManager, yes. But...it's still there in an OO design sense. So your LockManager implementation is forever bound (by a single teeny-weeny method call, granted) to the Data class/DBAccess interface.

Just off the top of my head, though, I wonder if by letting the LockManager not care about validity of the record but letting your Data class be the only one to know/care about could be made to work? After all, the Data class is very familiar with record validity and knows how to compute it...I wonder if we could help him keep that nasty secret to himself.

That way LockManager, lucky simple-minded young scoundrel he is, could dispense locking/unlocking calls all he wanted with nary a thought about records or valiidyt. Could it be made to be incumbent on the Data class to check...once the lock was obtained...as to whether the record was still valid or not? Once "owned", in a locked sense, then Data could make up it's own mind about what to do if he got an invalid record, like throw a RecordNotFoundException or something after unlocking it....?
</devils-advocacy>

I dunno....it just seems to me with the static reference to Data removed, you'd free up LockManager to be used by other classes outside DBAccess and it'd be one less little arrow I'd need to draw on my legal pad here. That's my biggest problem, no room left on the page.

...but if this makes no sense based on what you're got, ignore me. Remember, I'm still in the almost-no-design, certainly-no-code-blue-sky stage, so everything works correctly, cleanly and perfectly in my B&S system.

Good luck,
Richard

Damn that devil and his advocate...he's right again, which means more work for Alan

You're right, what I will do is have the Data.lock method which invokes LockManager.lock check if the record is still valid after the lock is received.
If not I will release the lock and throw a RecordNotFoundException.

Good catch.

Have you started thinking about deadlock avoidance as yet ?
If so any chance you could have a look at the conversation Yupp and I were having above and throw your two-cents into the mix.

You're a useful man to have around....as yet untouched by the evils of coding, so I can get a "pure" opinion off you

[Yupp Cook]

Originally posted by Alan Morgan:


Damn that devil and his advocate...he's right again, which means more work for Alan

You're right, what I will do is have the Data.lock method which invokes LockManager.lock check if the record is still valid after the lock is received.
If not I will release the lock and throw a RecordNotFoundException.

Good catch.

Have you started thinking about deadlock avoidance as yet ?
If so any chance you could have a look at the conversation Yupp and I were having above and throw your two-cents into the mix.

You're a useful man to have around....as yet untouched by the evils of coding, so I can get a "pure" opinion off you

Hi Alan

Fine.


That's similar to what I was trying to tell you in my postingin from February 12, 2006 04:38 PM

There s still some improvement for your lock workflow.
Now your plan is (correct me if I m wrong):
while isLocked(recNo) {
wait
}
lock(recNo)
ifNotValid(recNo) {
unlock(recNo)
RecNotFndExc
}

Improved:
while isLocked(recNo) {
wait
}
ifNotValid(recNo) {
RecNotFndExc
}
lock(recNo)

Both are synched on LockManager.
My advice is to check the validity before locking, so you will never lock a invalid record.


The deadlock problem:
I see problems getting a reliable client id which doesn't change under no circumstances.
So in my aproach the client could actually lock various records, but I prevent him from doing so not allowing further actions after a request was sent. I prever this way cause it s more flexible. What if one day booking of various records is favored. No major Data.java changes would be necessary then. Are there any problems I don't see?

Did I explain it well? I ve the feeling the more I work on this the more complex it gets. :roll:

Regards
Yupp

[Richard Brooks]

My advice is to check the validity before locking, so you will never lock a invalid record.

Couldn't thread-b mark a record deleted while thread-a was wait()ing for the lock so that while the record might have beenvalid before thread-a tried to get the lock but invalid after thread-a actually got the lock?

Richard

[Alan Morgan]

Yupp - The reason why I check record validity after the lock is that the code to check record validity is in the Data class. And to save me having a dependency from LockManager to Data I put the check in the Data class after the lock has been achieved.

Pseudo code Data.lock() as follows:

read record
LockManager.lockRecord
if(record not valid)
   unlock
   throw RecordNotFoundException
return lockCookie

The alternative is to put the Data.isValidRecord call into the LockManager.lock method but this is poorer from a design perspective.


On deadlock - I see your point about getting a consistent client id.
One question - if you did decide one day to allow locking of various records then where would you have to change your code ?


Thanks,
Alan.

[ March 15, 2006: Message edited by: Alan Morgan ]
[ March 15, 2006: Message edited by: Alan Morgan ]

[Alan Morgan]

Yupp - After having a read of this thread

https://coderanch.com/t/187162/java-developer-SCJD/certification/Dealing-Deadlocks-Stale-Locks

I got to thinking about what you said again.
In this thread John Smith says:

Originally posted by John Smith:
[b]In my implementation, the client can only lock one record at a time (the book() method would simply block until the record is booked).

So its seems to me (correct me if I am wrong) that he took the same approach as you.
One question - how exactly do you "block until the record is booked" ?

Oh nearly forgot - are you fat or thin client ?
So is your book() call on the server or client and do you think it makes a difference either way ?

Thanks again,

Alan.
[ March 15, 2006: Message edited by: Alan Morgan ]

[Yupp Cook]

Originally posted by Alan Morgan:
Yupp - After having a read of this thread

https://coderanch.com/t/187162/java-developer-SCJD/certification/Dealing-Deadlocks-Stale-Locks

I got to thinking about what you said again.
In this thread John Smith says:



So its seems to me (correct me if I am wrong) that he took the same approach as you.
One question - how exactly do you "block until the record is booked" ?

Oh nearly forgot - are you fat or thin client ?
So is your book() call on the server or client and do you think it makes a difference either way ?

Thanks again,

Alan.

[ March 15, 2006: Message edited by: Alan Morgan ]

HI Alan

It is the same approach! Having read the comments by John Smith encourages me.
I implemented a thin client architecture. I know you have a fat client, but the way to make sure only one record
can be booked at a time should be the same:

Let's simplify the book process to explain it easier. Clicking the book button calls the actionPerformed method.
Before this method ends (1) it sends the request AND (2) receives the result AND (3) updates the gui. Meanwhile no other action is technically possible - the client is blocked cause it's waiting for the method to return. That's all. It doesn't require any further programming effort.

Basically it looks like:
public void actionPerformed(ActionEvent e) {
Result result = book(infoFromGui);
client.setResult(result);
}

If one day I want to allow locking various records I'd have to seperate the 3 steps into different methods.
The actionPerformed would end before the result is back allowing the next locking action. The dead lock danger would arise.

Yupp

[Alan Morgan]

Ok Yupp I think I understand you now.

I believe mine works the same but I have to run some tests later to prove it.
So do you have a section in your choices.txt on deadlock ?
And if so do you say something simple like
"Deadlock is avoided as only 1 client can lock 1 record at any 1 time
This is acheived as the client blocks on each request...."

or something along these lines ?

Thanks again,
Alan.

[Alan Morgan]

Ok so I ran my tests and it seems that the blocking is working in the same way for me.

I put a bit of code into my server so that if you try to lock record 1 it hangs effectively.
So with the same client you can't do anything as the result is being waited on.
This tells me that if my lock/unlock occurs in one call from a client then a client can only lock one record at a time.

Just to check I had a second client connect to the server and try to lock record 2 and that worked fine.

How does this sound ?

[Yupp Cook]

Hi Alan

Originally posted by Alan Morgan:
Ok Yupp I think I understand you now.

I believe mine works the same but I have to run some tests later to prove it.
So do you have a section in your choices.txt on deadlock ?
And if so do you say something simple like
"Deadlock is avoided as only 1 client can lock 1 record at any 1 time
This is acheived as the client blocks on each request...."

or something along these lines ?

Thanks again,
Alan.

I'm about to finish BS2.1.2 but still haven't started the choices.txt. I ll try to explain how it's achieved looking at the inside of Data, my logic layer and the fact that the client only permits one request at a time.
Would be interested how others handle this..? :roll:

Originally posted by Alan Morgan:
Ok so I ran my tests and it seems that the blocking is working in the same way for me.

I put a bit of code into my server so that if you try to lock record 1 it hangs effectively.
So with the same client you can't do anything as the result is being waited on.
This tells me that if my lock/unlock occurs in one call from a client then a client can only lock one record at a time.

Just to check I had a second client connect to the server and try to lock record 2 and that worked fine.

How does this sound ?

Sounds good. I will test my server with numerous test clients all accessing the same records. But haven't done it yet.

[Animesh Saxena]

Since your Data class is singleton I guess ur read method is also synchronized. I was followin the same approach and I don't see any reason why It shouldn't be. You didn't write about it so i thought I will clarify...

If another client tries to read a record while someone is already reading he may end up changing some value of recNo passed to the read method and get an incorrect result.

public String[] read(int recNo) {
//skip DataOffset
.....
...

//reach The record
int absoluteOffset = recoNo * recordOffset;
Another thread may slice in here and change the Offset until u execute
dbFile.skipBytes(absoluteOffset);

}



Please let me know if I am wrong....

[Animesh Saxena]

Another thing that may be wrong is whenever u lock u try to synch on lockMap.
If another client wants to delete or update a different record he won't be able to lock it cos u have synch placed on lockMap.

[Alan Morgan]

Hi Animesh,

To answer you first question, yes I do synch on a read.
Not the whole method but the code where the RandomAccessFile is accessed.

Originally posted by Animesh Saxena:
Another thing that may be wrong is whenever u lock u try to synch on lockMap.
If another client wants to delete or update a different record he won't be able to lock it cos u have synch placed on lockMap.

True but they are not restricted from locking that record...they just have to wait until my lock operation runs which is a short time.
Perhaps I am missing something ?

Thanks,
Alan.

[Oricio Ocle]

Hello all
First of all, sorry for posting without reading the whole thread...
Alan, you are right:

True but they are not restricted from locking that record...they just have to wait until my lock operation runs which is a short time.

But think what happens in notification...
What do your specs says about locking methods?

[Alan Morgan]

Originally posted by Oricio Ocle:
Hello all
First of all, sorry for posting without reading the whole thread...
Alan, you are right:

But think what happens in notification...
What do your specs says about locking methods?

My spec says the following:

Your server must be capable of handling multiple concurrent requests, and as part of this capability, must provide locking functionality as specified in the interface provided above. You may assume that at any moment, at most one program is accessing the database file; therefore your locking system only needs to be concerned with multiple concurrent clients of your server. Any attempt to lock a resource that is already locked should cause the current thread to give up the CPU, consuming no CPU cycles until the desired resource becomes available.

Ok I synch on lockMap but if the record is already locked then I call wait which means that I give up my hold on lockMap so other clients can delete/update as they see fit.

In unlock I call lockMap.notifyAll so all threads waiting on it get a chance.
Granted you don't know which one will get it but they do get a chance.

I reckon I'm still missing something ?

[Oricio Ocle]

Ok think about that:

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.

Supose that:
Client A locks         record 1
Client B locks         record 2
Client C tries to lock record 1 -> Giving up CPU
Client B unlocks       record 2 -> NotifyAll waiting threads
Client C tries to lock record 1 -> Consuming CPU cycles

.... But it's record has not been unlocked ...

Regards

[Alan Morgan]

Originally posted by Oricio Ocle:
Ok think about that:

Supose that:
Client A locks         record 1
Client B locks         record 2
Client C tries to lock record 1 -> Giving up CPU
Client B unlocks       record 2 -> NotifyAll waiting threads
Client C tries to lock record 1 -> Consuming CPU cycles

.... But it's record has not been unlocked ...

Regards

My lock code has a while loop.
The first thing C does when he wakes up is to check if the record he is looking for is locked.
If it is he gives up the CPU again.

Does this solve the problem ?

[Oricio Ocle]

Hello again,
I think the only fact of entering in a running state makes the thread consume CPU cycles.
But IMO this could not lead to an automatic failure since it does not seem to be an explicit must. Anyway it's probable that it makes you lose some points in locking.
I lock on different instances to avoid that.
Regards