aspose file tools*
The moose likes Developer Certification (SCJD/OCMJD) and the fly likes NX:Client crashed cause deadlock in LockManager Big Moose Saloon
  Search | Java FAQ | Recent Topics | Flagged Topics | Hot Topics | Zero Replies
Register / Login
JavaRanch » Java Forums » Certification » Developer Certification (SCJD/OCMJD)
Bookmark "NX:Client crashed cause deadlock in LockManager" Watch "NX:Client crashed cause deadlock in LockManager" New topic
Author

NX:Client crashed cause deadlock in LockManager

Kruger Scheitz
Ranch Hand

Joined: Jul 31, 2002
Posts: 72
my lock implementation look something like this
public void lock( record, owner )
{
while( dblock ) {
wait();
}
if( record == -1 )
dblock = true;
}
this is what i'm trying to do.
Whenever a request to lock the whole datebase comes in, I set a boolean to true and if there are any currently locked records I call wait(), I don't allow any more records to be locked, and allow the currently locked records to be unlocked before granting a whole database lock.
Now when clients crashed after calling lock() without call unlock(), it will cause the record to be locked for ever. How to fix this deadlock?
Mark Spritzler
ranger
Sheriff

Joined: Feb 05, 2001
Posts: 17257
    
    6

Look at the Unreferenced Interface. This will allow you to unlock records from a client that has crashed.
Also to note, the only call to lock for locking the entire database will come from the server when it is shutting down. No client should ever call for a lock of the entire database.
Mark


Perfect World Programming, LLC - Two Laptop Bag - Tube Organizer
How to Ask Questions the Smart Way FAQ
S. Ganapathy
Ranch Hand

Joined: Mar 26, 2003
Posts: 194
Hi Mark,
Is it must to implement Unreferenced Interface for remote objects?
Is it so necessary to implement the interface in our assignemnt(RMI server)?
If WeakHashMap is used to store locks, after sometime, if there is no client reference, an entry in a WeakHashMap will automatically be removed when its key is no longer in ordinary use. Will this WeakHashMap help?
Please help me.
Regards,
Ganapathy.
[ July 01, 2003: Message edited by: S. Ganapathy ]
Andrew Monkhouse
author and jackaroo
Marshal Commander

Joined: Mar 28, 2003
Posts: 11484
    
  94

Hi Ganapathy
The problem with WeakHashMap is that your code does not get notified when an object is removed.
Consider when a client (Client 'A') attempts to get a lock, and finds that it is already locked, so it calls wait(). The client that does have this record locked (Client 'B') crashes, so sometime later the reference to this client is removed from the WeakHashMap. But what happens to the client currently waiting for the lock (Client 'A')? The reference has been silently removed from the WeakHashMap, so notify() has not been called.
You could write code which will handle the possibility of locks disappearing, and as long as you do so, you should be OK with WeakHashMaps.
This is where I see the advantage of Unreferenced: Your code is explicitly called when an object becomes unreferenced, so you have more chance of running "normal" clean up operations, such as unlocking any locked records (which should be calling notify(), so your waiting clients get woken up). Of course the disadvantage is that unreferenced() can be called multiple times for the one object, so you have to be careful how you write your code so that it does not cause an infinite loop.
Regards, Andrew


The Sun Certified Java Developer Exam with J2SE 5: paper version from Amazon, PDF from Apress, Online reference: Books 24x7 Personal blog
S. Ganapathy
Ranch Hand

Joined: Mar 26, 2003
Posts: 194
Hi Andrew,
I am providing locking functionality in LockManager class. This class holds the record number, and returns a cookie value, which are stored in HashMap object. Locking mechanism in my assignment is entirely at the server side.
Any operation in server side, like update record, is a three step process. First lock the record, then update the record and then unlock the record. Everything is done from single method, but it is not an atomic operation.
Do I need to take care of client crashes?
Once client sends the record to the server, and even if client crashes, as server receives the data, it will compete the operation of lock-update-unlock.
Please advice me.
Regards,
Ganapathy.
Andrew Monkhouse
author and jackaroo
Marshal Commander

Joined: Mar 28, 2003
Posts: 11484
    
  94

Hi Ganapathy
Everything is done from single method, but it is not an atomic operation.

Since it is not atomic, then I think it would be possible for an orphaned lock to be produced. Granted, it would be difficult, but it is possible. The situation could occur if the client crashes while it's remote code is in wait() state. If the remote code does not get notified of lock availability until moments before Java notices that the client is no longer connected, then it might have time to get the lock, then get killed by the JVM.
Regards, Andrew
S. Ganapathy
Ranch Hand

Joined: Mar 26, 2003
Posts: 194
Hi Andrew,
Once the request goes to the server, even if there is wait(), the server method waits, to lock the record, once it locks the record, further processing will be done like update the record. Even if the update fails, lock on record will get released, and client will get notified. If client already crashes, still server completes the operation. Either updates, or throwing exception.
Is there any insecurity in this process? This is my worry.
Regards,
Ganapathy.
Andrew Monkhouse
author and jackaroo
Marshal Commander

Joined: Mar 28, 2003
Posts: 11484
    
  94

Hi Ganapathy,
The example with the wait state was just an example of how the potential problem I forsee could manifest itself.
The problem is that you said that your method is not atomic. This implies that the following can occur:
  • your method calls lock()
  • the JVM switches out your method while it runs another task
  • your method does any verification necessary
  • the JVM switches out your method while it runs another task
  • your method calls update()
  • the JVM switches out your method while it runs another task
  • your method calls unlock()
    Now, as I said, in most cases this will be fine. But there is the possibility that in one of those instances where the JVM switched out your method, that it decided that the client is dead, and stops your processing.
    Regards, Andrew
  • Philippe Maquet
    Bartender

    Joined: Jun 02, 2003
    Posts: 1872
    Hi,
    Perhaps I invested too much in my LockManager manager class, but I wanted it to guard myself against any lock issue, as those mentioned in a few threads here :
    - risk of deadlocks if I let the user book multiple rooms at once as shown in this pseudo-code :
    locking multiple records (risk of deadlock)
    if ok reading the records
    if ok, update them
    finally unlock records really locked
    - risk of deadlocks if a client crashes (even if multiple-record-locking is not implemented)
    So my LockManager class supports :
    . deadlock detection with automatic repair
    . automatic unlocking on timeout
    . automatic unlocking of records locked by dead threads
    . reentrant locking
    It works fine and I think it was worth.
    Regards,
    Philippe MAQUET.
    George Fung
    Ranch Hand

    Joined: Jun 12, 2003
    Posts: 98
    Hi Philippe,
    Could you tell me how to do the following stuffs?
    . deadlock detection with automatic repair
    . automatic unlocking on timeout
    . automatic unlocking of records locked by dead threads
    . reentrant locking


    SCJP, SCJD, SCWCD, SCBCD, SCEA, SCJP6
    To be obtained: SCEA 5
    Philippe Maquet
    Bartender

    Joined: Jun 02, 2003
    Posts: 1872
    Hi George,
    Why not ? :-)
    Here is my lockRecord method (pseudo-code) :


    As far as "automatic unlocking on timeout" and
    "automatic unlocking of records locked by dead threads" are concerned, it was quite simple to be implemented by a maintenance thread which tests periodically (every second in my implementation) that :
    . every lock is owned by a isAlive() thread ;
    . every lock is not owned for more then my lockTimeout property,
    and in either case unlock those invalid locks.
    I hope it was clear. If any of you see an issue, please tell me.
    Phil.
    [ July 07, 2003: Message edited by: Philippe Maquet ]
    [ July 08, 2003: Message edited by: Philippe Maquet ]
    Andrew Monkhouse
    author and jackaroo
    Marshal Commander

    Joined: Mar 28, 2003
    Posts: 11484
        
      94

    Hi Philippe
    I chose to ignore deadlocks in my assignment, as I believed it was out of scope. None of my clients would ever attempt to gain two locks at the same time, so deadlock could not occur with my code. And I documented that decision. (But I did loose four marks in my server, so who knows - it may have been a bad decision :roll: )
    Your deadlock prevention will still allow a deadlock if three or more clients are doing locking at once:
  • client 'A' locks record 1 and attempts to lock record 2
  • client 'B' locks record 2 and attempts to lock record 3
  • client 'C' locks record 3 and attempts to lock record 1
    Do you document this?

  • As far as "automatic unlocking on timeout" and "automatic unlocking of records locked by dead threads" are concerned, it was quite simple to be implemented by a maintenance thread which tests periodically (every second in my implementation) that :
  • every lock is owned by a isAlive() thread ;
  • every lock is not owned for more then my lockTimeout property,

  • and in either case unlock those invalid locks.

    What do you do if the lockTimeout property is exceeded?
    I am not sure about you throwing RuntimeExceptions for DeadLock (and presumably Timeout). I would think that client code should be able to handle both conditions, but since the client does not have to catch a RuntimeException, a client application could die a horrible death if the programmer did not catch your exception.
    The solution that I see is that both these exceptions are only relevant in the networked code, and in the networked code you could throw exceptions that are not listed in Sun's interface, so for the networked code you could have some checked exceptions for deadlock and timeout.
    Regards, Andrew
    Andrew Monkhouse
    author and jackaroo
    Marshal Commander

    Joined: Mar 28, 2003
    Posts: 11484
        
      94

    Hi Philippe
    Also, I am unsure about your looking at thread death as a way of determining if the client has died. I guess it depends on what thread you are using. Just using the thread running your remote class is not really valid though, as there is no guarantee that two calls to remote methods from the one client will run in the same thread.
    Take a look at this code:
    Running it gives me the following output:

    Even though the client still had a valid connection, I still got notified of the first thread death.
    (I had been playing with setting the lease value, but doing this seemed to change how long the thread stayed active for (both calls to the remote method ran in the same thread, even with a 60 second delay) so I commented out that code).
    Regards, Andrew
    Philippe Maquet
    Bartender

    Joined: Jun 02, 2003
    Posts: 1872
    HI Andrew,

    I chose to ignore deadlocks in my assignment, as I believed it was out of scope. None of my clients would ever attempt to gain two locks at the same time, so deadlock could not occur with my code.

    Whichever point of view I take, I believe that deadlock prevention is a must :
    As the URLyBird application writer :
    ----------------------------------
    The specs say that "the user interface must allow the user to book a selected record, updating the database file accordingly."
    What does mean "a" (selected record) ? Does it mean "one" or "a" among a few ?
    I think that booking hotel rooms is typically a transactional activity : if you - as a customer - try to organize a 3 days-2 nights city-trip, you'll want to book either 2 rooms or none.
    So I'll let the user multi-select records to book them all in a single transaction.
    As the Data class writer :
    ------------------------
    I claim that I implement the DBAccess interface, which in turn allows locking multiple records by a same thread. I cannot make any assumption about how other programmers (perhaps in other applications) will use it. Simply documenting that deadlock situations are not handled is not enough, because it's like you write something like "Warning : using the Data class as described in its interface could lead your application to hang indefinitely." :-)
    Another approach could be to keep deadlock prevention as a further improvement and in the meantime prevent any thread to own more than one lock. I had 2 reasons not to do so (the first is good, the second very bad) :
    1� As when you build a house, some features are difficult to add later, if they relate to the foundations, and that's the case here.
    2� It was so exciting and fun to design and code it ! :-)

    Your deadlock prevention will still allow a deadlock if three or more clients are doing locking at once:
    client 'A' locks record 1 and attempts to lock record 2
    client 'B' locks record 2 and attempts to lock record 3
    client 'C' locks record 3 and attempts to lock record 1

    Well, thank you *very* *very* much for pointing this out. I will not document it because I hate to document bugs, I prefer to correct them ! :-)
    If such locking situation arise in my current code, the deadlock will not be prevented but well broken by my maintenanceThread when the blocking lock times out. It means that the application will not hang, but it's not acceptable for all that.
    Here is the situation you describe :

    When C wants to get a lock on record 1 which is not available immediately, instead of asking to himself :
    "Am I the owner of the lock the blocking thread is waiting for ?",
    it must ask the following question :
    "Am I the owner of the lock the blocking threads chain is waiting for ?"
    It's easy to correct by recursivity :
    C wants 1 locked by A waiting for 2 locked by B waiting for 3 locked by ... me >> deadlock
    Thanks again !

    What do you do if the lockTimeout property is exceeded?

    The maintenanceThread unlocks the record. Of course, there is no exception thrown, because there would be nobody to catch and handle it (except the JVM ! :-)).
    In fact, you get the same situation as if the lock had never been granted. It means that when the thread who "thinks" owning the lock will try to use it, he will receive a SecurityException. But he can prevent it by testing its lock with the boolen isLockOwned(recNo) method.

    I am not sure about you throwing RuntimeExceptions for DeadLock (and presumably Timeout). I would think that client code should be able to handle both conditions, but since the client does not have to catch a RuntimeException, a client application could die a horrible death if the programmer did not catch your exception.

    As I explained above, I have no TimeoutException.
    When LockManager detects a deadlock, it's common sense that he must communicate it to the caller. As DBAccess interface cannot be changed (no new checked exceptions are allowed), I see only 3 ways to do it :
    1� From the long lockRecord(long recNo), return a special cookie value (0 by example) : it'd been my preferred solution in 1986 when I was programming in Clipper... :-)
    2� Use the new exception chaining facility : wrap my DeadLockException as the cause of the checked RecordNotFoundException, and throw the latter instead. I feel that's as stupid as solution 1�.
    3� Throw my DeadLockException (runtime) and document it with the @throws javadoc tag. I don't like it, but it's still better than 1� and 2�.
    I am a Java newbie. So if you think that I am wrong, please be kind to let me know.

    Also, I am unsure about your looking at thread death as a way of determining if the client has died. I guess it depends on what thread you are using. Just using the thread running your remote class is not really valid though, as there is no guarantee that two calls to remote methods from the one client will run in the same thread.

    I use sockets and make sure that a client connection keeps the same thread. But of course, I will document that locks cannot spread over multiple spread, and explain why.
    Thanks again for your comments. They were helpful !
    Regards,
    Phil.
    [ July 04, 2003: Message edited by: Philippe MAQUET ]
    Andrew Monkhouse
    author and jackaroo
    Marshal Commander

    Joined: Mar 28, 2003
    Posts: 11484
        
      94

    Hi Philippe
    I like your reasoning for the deadlock prevention. I have used many of the same arguments to explain why I believe that the client should be able to call the lock() and unlock() methods directly, rather than calling a modify() method on the server.

    When LockManager detects a deadlock, it's common sense that he must communicate it to the caller. As DBAccess interface cannot be changed (no new checked exceptions are allowed), I see only 3 ways to do it :

    I agree that option 1 is not OO, and option 2 is just plain ugly. Which leaves option 3.
    Just to give you another option to think about though. I think you are coding such that:

    This way round you are correct - you are limited by the exceptions that may be thrown by the Data class.
    Turn it around though, and you loose that restriction:

    This has other issues though. And the one that I think you will hate is that this means that anyone accessing the Data class directly will loose all the work you have put into the LockManager.
    Regards, Andrew
    [ July 04, 2003: Message edited by: Andrew Monkhouse ] (I got caught by that UBB bug where editing a post with quotes goes crazy!)
    [ July 04, 2003: Message edited by: Andrew Monkhouse ]
    Philippe Maquet
    Bartender

    Joined: Jun 02, 2003
    Posts: 1872
    Hi Andrew,
    I have used many of the same arguments to explain why I believe that the client should be able to call the lock() and unlock() methods directly, rather than calling a modify() method on the server.

    Nothing in my assignment prevents neither force me to expose the Data lock/unlock methods to the client, but I decided not to do so after weighting pros and cons :
    pros : I don't see any :-)
    cons : scattering "functionally atomic" operations (such as booking rooms) among multiple network connections is
  • less reliable
  • less efficient


  • This has other issues though. And the one that I think you will hate is that this means that anyone accessing the Data class directly will loose all the work you have put into the LockManager.

    You're right, taking into account my global design decisions as far as data access is concerned :
  • My Data class is a multiton, in order to be sure that no more than one instance per database file exists in a single JVM. The only way to get a Data instance is to use its static method Data getInstance(String dataFileName).
  • LockManager is not public and is a multiton too. That design allows locks to be managed (including deadlock prevention) accross multiple table, which is classic. Each Data instance gets its LockManager reference by calling the static LockManager getInstance(Data data).


  • I hope the whole stuff seems consistent.
    Regards,
    Philippe.
    Vlad Rabkin
    Ranch Hand

    Joined: Jul 07, 2003
    Posts: 555
    Hallo Andrew,
    Hallo Philippe,
    Could you help me understand the ideas about Unreferenced or WeakHashMap.
    Let's take WeakHashMap:
    My key is Record Number (Wrapped int).
    It is my constructor of Data:
    public Data(String fileName, LockManager lockManager)
    My C-tor for RemoteData:
    public DataRemote(DB db, ReadWriteLock rwManager)
    In DataRemote in lock method I do have following:
    cookie = db_.lock(recNo);
    and in unlock:
    db_.unlock(recNo, cookie);
    , where recNo is int.
    So, actually my DataRemote has no direct reference on Key (Integer(recNo)).
    It has only reference on Data.
    If I do WeakHashMap, lock will always suddenly dissapear! ??? Is it right? If so, how should I do it?
    Thanx,
    Vlad
    Philippe Maquet
    Bartender

    Joined: Jun 02, 2003
    Posts: 1872
    Hi Vlad,
    Unfortunately, I cannot be of much help currently as far as Unreferenced and WeakHashMap are concerned :
  • Unreferenced is a RMI topic and as I chose sockets, I didn't dig into RMI that much.
  • I am not a WeakHashMap specialist, as that basic question (still pending BTW) shows in : NX: URLyBird (Database Schema Parser)


  • Maybe Andrew ...
    Cheers,
    Phil.
    Vlad Rabkin
    Ranch Hand

    Joined: Jul 07, 2003
    Posts: 555
    This question in the topic pointed by U is irrelevant for me, because my Data is Singleton.
    Anyway, thanx!
    Max Habibi
    town drunk
    ( and author)
    Sheriff

    Joined: Jun 27, 2002
    Posts: 4118
    Originally posted by Vlad Rabkin:
    Hallo Andrew,
    Hallo Philippe,
    Could you help me understand the ideas about Unreferenced or WeakHashMap.
    Let's take WeakHashMap:
    My key is Record Number (Wrapped int).
    It is my constructor of Data:
    public Data(String fileName, LockManager lockManager)
    My C-tor for RemoteData:
    public DataRemote(DB db, ReadWriteLock rwManager)
    In DataRemote in lock method I do have following:
    cookie = db_.lock(recNo);
    and in unlock:
    db_.unlock(recNo, cookie);
    , where recNo is int.
    So, actually my DataRemote has no direct reference on Key (Integer(recNo)).
    It has only reference on Data.
    If I do WeakHashMap, lock will always suddenly dissapear! ??? Is it right? If so, how should I do it?
    Thanx,
    Vlad

    Almost correct. In the WeakHashMap, your key is the reference to the client, and your value is the Integer wrapped recno. That way, when the client crashes, RMI will eventually release that client. When they are released, then the WeakHashmap will release the record.
    Alternately, you could implement Unreferenced. That way, when the client crashes, the Unreferenced method will be called, In this scenario, you can write code that explicitly release the lock.
    Both are valid solutions, and both offer challenges. The problem with un referenced, IMO, is twofold. First, it's more complicated(you have to write extra code: WeakHashMap doesn't require any code at all). Second, it's possible that Unreferenced will be called several times, because the reference count can drop to zero, then back up again, then back to zero. Unreferenced, of course, is called whenever the reference count hits zero, not just the first time.
    WekaHashMap also has a theoretical problem, because notifyAll could never be called on the map when the client is released. Thus, if you have a very high number of clients, it's possible that the normal scheduling happens too quickly for a new thread to pick up on the fact that the map is free. I have never been able to produce this behavior, but it's possible. Either way, it's a toss up. IMO, WeakHashMap is a little better, but other reasonable opinions differ.
    All best,
    M


    Java Regular Expressions
    Vlad Rabkin
    Ranch Hand

    Joined: Jul 07, 2003
    Posts: 555
    Hi Max,

    In the WeakHashMap, your key is the reference to the client, and your value is the Integer wrapped recno.

    WAW, I don't understand it ...
    Where do U keep ypur cookies then?
    My Key is ID(Integer) and my Value is Lock object (This class wrappes only the long cookie).
    And what you mean id is client? What class/object do U mean?
    Thanx,
    Vlad
    Max Habibi
    town drunk
    ( and author)
    Sheriff

    Joined: Jun 27, 2002
    Posts: 4118
    Originally posted by Vlad Rabkin:
    Hi Max,


    WAW, I don't understand it ...
    Where do U keep ypur cookies then?

    In a box under the bed, so my wife doesn't throw them away .

    My Key is ID(Integer) and my Value is Lock object (This class wrappes only the long cookie).
    And what you mean id is client? What class/object do U mean?
    Thanx,
    Vlad

    Let me back up a bit: Can you lay out your design in a bit more detail?
    M
    Vlad Rabkin
    Ranch Hand

    Joined: Jul 07, 2003
    Posts: 555
    Hi Max,

    Let me back up a bit: Can you lay out your design in a bit more detail?

    So, just have changed the design a bit:
    I use RMI.
    I have a Factory (is published in RMI Registry) which delivers one remote object pro Client (So, it is not a Singelton). The main method of it creates LockManager(Singelton), Data(Singelton)and ReadWriteLock(Singelton)
    Every time getConnection() on Factory from client is called a new DataRemote Object is created and it gets (via C-tor) LockManager object, Data object and ReadWriteLock object.
    Now, Lockmanager has internally synchronized HashMap. This HashMap holds RecordNumbers (int) (wrapped in Integer) as its Keys and cookies (wrapped in a my own simple class called Lock).
    LockManager provides 3 methods:

    That's it.
    So, the lock have nothing directly to do with clients or RemoteData object, they are dealing only with "client threads" on the server and Record Numbers, which are locked.
    I hope I shouldn't completely reconsider my design to us Unreferenced or WeakHashMap
    Thanks you much in advance...
    Vlad
    Max Habibi
    town drunk
    ( and author)
    Sheriff

    Joined: Jun 27, 2002
    Posts: 4118
    Vlad,
    Given you design, I think that either Unreferenced or a WeakHashMap could help you, and fairly easily @ that. For WeakHashMap, what you want, I think, is to pass in the recno from the unique remote client to the LockManager, who'll just hold on to it. It's important that the Integer(recNo) actually be an Integer that's created by the unique remote RMI object, and then passed in as a paramater. Why? Because when that remote object goes out of scope, it's member variables will be gc'd. When that happens, the recno be be removed, releasing the int.
    So, basically,
    1. use a Weakhashmap instead of a HashMap,
    2. Make sure that your lock take an Integer as a pram, instead of taking an int and wrapping i on the fly.
    HTH
    Vlad Rabkin
    Ranch Hand

    Joined: Jul 07, 2003
    Posts: 555
    Ok, Max
    Thanx a lot!
    Vlad
    Jim Yingst
    Wanderer
    Sheriff

    Joined: Jan 30, 2000
    Posts: 18671
    Mmmm, I don't think the WeakHashMap is as clean a fit here as Max has implies.
    2. Make sure that your lock take an Integer as a pram, instead of taking an int and wrapping i on the fly.
    This is a problem - the lock() method defined in the API we must implement required an int, not Integer. We can't change that. I suppose we could provide an overloaded lock() method that takes an Integer and is not part of the DB interface (called DBMain in some assignments, I think). But that feels wrong to me, to have a method in the DB interface that is almost what we need, and then we don't use it at all, instead relying on another method. If we're going to do that, why not give Data some additional methods that just omit cookies entirely? After all, they just get in the way of a clear, simple program.
    Seriously, my Data does provide some additional methods not in DB - notably getMetaData() which allows me to avoid hardcoding column names and lengths. But I try to use the previously specified DB methods wherever possible, which in this case means using lock(int) rather than lock(Integer). So this solution seems unsatifactory to me. Furthermore I worry about those pesky junior programmers, who may not really understand why they must use lock(Integer) rather than lock(int). (And I hope they don't decide to use that same Integer anywhere else - though that seems unlikely.) I'm thinking the WeakHashMap approach makes more sense for people doing an assignment without cookies (there are some like that, right?).
    My own solution is: A ConnectionFactory returns Connection instances, one per client connection. Connection is an interface whose server-side implementation is ConnectionImpl. Each ConnectionImpl carries its own HashMap or the record numbers and cookies pertaining to the locks held by that particular connection[/b]. Connection has a close() method which iterates though the map and closes all associated connections. This close() method can be called by both unreferenced() [i]and finalize(). I may not really need both, but why not; it can't hurt. The close() method is written in such a way that it doesn't matter if it's called more than once.
    It does feel a bit cumbersome for each Connection to have its own HashMap. They were previously very lightweight objects; now they're a bit heavier. If we had a lot of clients and were worrying about memory usage and performance, this could be a problem. But there's little justification for that particular concern now, IMO. In the real world, I'd advise the customer to just get rid of those dame cookies, and the problem will go away entirely.
    Both are valid solutions, and both offer challenges. The problem with un referenced, IMO, is twofold. First, it's more complicated(you have to write extra code: WeakHashMap doesn't require any code at all).
    A valid concern in general. Though I think there's a certain advantage in having cleanup code appear explicily in our classes rather than being handled behind the scenes. With explicit code, junior programmers will at least know that it's there. It's isolated in one class; easy enough to skip past unless/until it's necessary to modify the cleanup somehow.
    Second, it's possible that Unreferenced will be called several times, because the reference count can drop to zero, then back up again, then back to zero. Unreferenced, of course, is called whenever the reference count hits zero, not just the first time.
    Interesting point. I'm thinking this is a non-issue for my code, but want to check my understanding. How would the reference count increase after it's hit zero? The main way that comes to mind is if the instance is bound to the registry - any clint that does Naming.lookup() with the appropriate name will get a new cleint reference to the object, no matter how many other client refereces exist. So OK, beware of making a bound object Unreferenced, as the unreferenced() method may be called multiple times. But with the ConnectionFactory, only the factory is bound. Each Connection instance returned by the factory's connect() method is a brand new instance, and only one client reference exists - from the client who did the connect(). In this context, a call to unreferenced() seems pretty unambiguous - that client is done, period, and won't be called again. Does that make sense? Is there some other mechanism through which the count of remote references could increase from zero? My experience with RMI is pretty limited, so I could well be overlooking something.


    "I'm not back." - Bill Harding, Twister
    Max Habibi
    town drunk
    ( and author)
    Sheriff

    Joined: Jun 27, 2002
    Posts: 4118
    /*
    Originally posted by Jim Yingst:
    Mmmm, I don't think the WeakHashMap is as clean a fit here as Max has implies.

    It has it's own problems, to be sure: nothing is prefect. One of these is that fact the even as an element is removed, the map might not get notifyAll called on it(as stated above, I think). However, this has nothing to do with a weakHashMap per se: it's just the nature of anything that takes advantage of both synchronization and Unreferenced at the same time. Your own code is limited by this. That is, if your Unreferenced code isn't synchronized on the hashmap, then it won't notify the map when you quitely remove an element. OTOH, if it is synchronized on the Map, it might never get called, because a client that's holding on the lock might die without ever calling notifyAll().

    2. Make sure that your lock take an Integer as a pram, instead of taking an int and wrapping i on the fly.

    This is a problem - the lock() method defined in the API we must implement required an int, not Integer. We can't change that.

    Nor should we: However, that's not the map Vlad and I were talking about. Vlad isn't talking about Data, he's talking about his lock manager.

    Now, Lockmanager has internally synchronized HashMap.


    I suppose we could provide an overloaded lock() method that takes an Integer and is not part of the DB interface (called DBMain in some assignments, I think). But that feels wrong to me, to have a method in the DB interface that is almost what we need, and then we don't use it at all, instead relying on another method. If we're going to do that, why not give Data some additional methods that just omit cookies entirely? After all, they just get in the way of a clear, simple program.
    Seriously, my Data does provide some additional methods not in DB - notably getMetaData() which allows me to avoid hardcoding column names and lengths. But I try to use the previously specified DB methods wherever possible, which in this case means using lock(int) rather than lock(Integer). So this solution seems unsatisfactory to me. Furthermore I worry about those pesky junior programmers, who may not really understand why they must use lock(Integer) rather than lock(int). (And I hope they don't decide to use that same Integer anywhere else - though that seems unlikely.) I'm thinking the WeakHashMap approach makes more sense for people doing an assignment without cookies (there are some like that, right?).
    My own solution is: A ConnectionFactory returns Connection instances, one per client connection. Connection is an interface whose server-side implementation is ConnectionImpl. Each ConnectionImpl carries its own HashMap or the record numbers and cookies pertaining to the locks held by that particular connection[/b]. Connection has a close() method which iterates though the map and closes all associated connections. This close() method can be called by both unreferenced() [i]and finalize(). I may not really need both, but why not; it can't hurt. The close() method is written in such a way that it doesn't matter if it's called more than once.

    This is where the Weakhashmap could save you a lot of trouble, I think. I like the ConnectionFactory: it's makes a lot ofsense. But. Simple have all of those remote connections share the same WeahHashMap(instead of a private HashMap), and presto, you're done. The extra code you're writing for Unreferenced, as well the code guaranting that the close() method is safe if called called twice, as well the finalize call all go away. You don't need them at all anymore. Thus, no more complexity then necessary, but exactly as much functionality. A given client will have an instance of Data as a member variable. Data, in turn, will have the static WeakHashMap.

    It does feel a bit cumbersome for each Connection to have its own HashMap.

    I don't think I understand what you're doing here. Can you elaborate a bit about what's in these maps?


    They were previously very lightweight objects; now they're a bit heavier. If we had a lot of clients and were worrying about memory usage and performance, this could be a problem. But there's little justification for that particular concern now, IMO. In the real world, I'd advise the customer to just get rid of those dame cookies, and the problem will go away entirely.
    Both are valid solutions, and both offer challenges. The problem with un referenced, IMO, is twofold. First, it's more complicated (you have to write extra code: WeakHashMap doesn't require any code at all).
    A valid concern in general. Though I think there's a certain advantage in having cleanup code appear explicitly in our classes rather than being handled behind the scenes. With explicit code, junior programmers will at least know that it's there. It's isolated in one class; easy enough to skip past unless/until it's necessary to modify the cleanup somehow.

    There's stuff going behind the scenes anyway: the gc, the dgc, the callbacks for Unreferenced, etc. IMO, and again, only MHO, the weakHashMap is a discreet bundle of isolation, written by Sun, that does everything we need here. I'm not sure I'm motivated to duplicate it's functionality.


    Second, it's possible that Unreferenced will be called several times, because the reference count can drop to zero, then back up again, then back to zero. Unreferenced, of course, is called whenever the reference count hits zero, not just the first time.
    Interesting point. I'm thinking this is a non-issue for my code, but want to check my understanding. How would the reference count increase after it's hit zero? The main way that comes to mind is if the instance is bound to the registry - any clint that does Naming.lookup() with the appropriate name will get a new cleint reference to the object, no matter how many other client refereces exist.

    RMI may decide to recycle the object to another client. Try this. Write 2 remote threads, and give the object to one of them. Then, after the client releases the object, ref count hits zero. Now, give the object to the second client. When that second thread is finished(caput, dead, no longer with us, visiting Elvis), the ref count will again hit zero, and Unreferenced will again be called. The problem here is that you have to write fairly non-junior-level code to provide functionality that the user didn't ask for. Further, from an aesthetic point if view, you've limited the salability of your architecture, because your built in architectural premise is that each client gets a unique connection. If that premise breaks, you're going to be wanting a paddle.

    So OK, beware of making a bound object Unreferenced, as the Unreferenced() method may be called multiple times. But with the ConnectionFactory, only the factory is bound. Each Connection instance returned by the factory's connect() method is a brand new instance, and only one client reference exists - from the client who did the connect(). In this context, a call to Unreferenced() seems pretty unambiguous - that client is done, period, and won't be called again. Does that make sense? Is there some other mechanism through which the count of remote references could increase from zero? My experience with RMI is pretty limited, so I could well be overlooking something.

    I think the scenario, as described above, would do it. Unless I'm misunderstanding your question. Let me put it this way: your architecture, as it stands, will work(IMO, of course). but(again, IMO), it has more code then it needs to, more complexity then it needs to, and less scalability then it needs to. But, again, this is just my aesthetic, and it's certainly not valid then your own, not even to me .
    All best,
    M
    Jim Yingst
    Wanderer
    Sheriff

    Joined: Jan 30, 2000
    Posts: 18671
    [JY]: This is a problem - the lock() method defined in the API we must implement required an int, not Integer. We can't change that.
    [MH]: Nor should we: However, that's not the map Vlad and I were talking about. Vlad isn't talking about Data, he's talking about his lock manager.

    OK, I guess I need to understand this better. Does Data still have lock(int) and unlock(int, long) methods? Are they still used? Are they called by the LockManager, or do they call the LockManager's methods? Which object is actually enforcing the locks, and which is just forwarding calls? If a Client calls LockManager.lock(Integer), does that create a call to Data.lock(int)? Or is the record already locked once LockManager.lock(Integer) is called? If it's the latter, isn't this circumventing Data's API entirely (as far as locking is concerned at least)? I'm so confused...
    ----
    Since I'm not sure I'm understanding how Vlad or Max's designs work here, I'll just talk about my own design while hoping for clarification. Swapping the order around a bit here...
    [MH]: I don't think I understand what you're doing here. Can you elaborate a bit about what's in these maps?
    Each Connection object has an internal HashMap of its own. The keys are Integers representing records that have been locked by that Connection; the values are Longs representing the cookie values that would be needed to unlock those records. To clarify: this data structure has nothing to do with enforcing locks (or the wait/notify that goes with them). That's handled by another HashMap inside Data, with entries for all locked records. The Connection's HashMap just helps the Connection know how to unlock any records it had previously locked, in case of disconnect. The Connection still needs to make a call to Data to actually perform the unlock().
    [MH]: Your own code is limited by this. That is, if your Unreferenced code isn't synchronized on the hashmap, then it won't notify the map when you quitely remove an element. OTOH, if it is synchronized on the Map, it might never get called, because a client that's holding on the lock might die without ever calling notifyAll().
    Sorry, I don't understand this. For what it's worth, I do access the HashMap from synchronized code, because RMI may allow multiple threads to be associated with a single Connection, and so I must protect mutable shared data. But why do we care about notify()/notifyAll() in here? Here's a simplified view of my close() method:

    It's not performing any wait(), and so it doesn't need to receive any notify() or notifyAll() in order to function. You're not saying we need to receive a notify() in order to enter a synchronized block, are you? Or are you referring to the fact that other threads must receive a notify() if they were wait()ing for a record that the Connection unlocked? That's taken care of by db.unlock(). The db's lock() and unlock() implement a wait/notify, but that's using a separate monitor abstracted away inside the db instance. Not really relevant here, IMO.
    ----
    [MH]: There's stuff going behind the scenes anyway: the gc, the dgc, the callbacks for Unreferenced, etc. IMO, and again, only MHO, the weakHashMap is a discreet bundle of isolation, written by Sun, that does everything we need here. I'm not sure I'm motivated to duplicate it's functionality.
    OK, very valid point.
    ----
    [MH]: RMI may decide to recycle the object to another client. Try this. Write 2 remote threads,
    Is that two threads in the same remote JVM, or separate JVMs?
    and give the object to one of them.
    "Give" how? Are you talking about an object which has been bound, which any thread in a remote client can "get" with Naming.lookup()? That's what I do with my ConnectionFactory. Or are you talking about passing a remote object as a return value in a remote method invocation, as when ConnectionFactory.connect() returns a new Connection? Or are you talking about two threads in the same remote JVM passing a reference between them which refers to a single remote Connection_Stub instance on that remote JVM? Or some other means of "giving"?
    Then, after the client releases the object, ref count hits zero. Now, give the object to the second client.
    And here's where the method of "giving" is critical. If it's done via Naming.lookup() then yes the reference count went to zero and back, as I already discussed. If it's done using the return value of a remote method, then we have to look at where that method got the object. In the case of my ConnectionFactory's connect() method, it creates a new ConnectionImpl() every time it's called. The ConnectionImpl that the second thread talks to is not the same object that the first thread talked to. The new object has a reference count of 1, while the original object still has a reference count of 0. Or perhaps you mean something else?
    When that second thread is finished(caput, dead, no longer with us, visiting Elvis), the ref count will again hit zero, and Unreferenced will again be called. The problem here is that you have to write fairly non-junior-level code to provide functionality that the user didn't ask for.
    Well I still don't see how close() would be called again, but if it is, the map.clear() at the end ensures it won't attempt to unlock those records a second time. Doesn't seem too horribly non-junior-level.
    Further, from an aesthetic point if view, you've limited the salability of your architecture, because your built in architectural premise is that each client gets a unique connection. If that premise breaks, you're going to be wanting a paddle.
    Actually I've been assuming that muliple clients might eventually want to share a connection, and my Connection objects are indeed thread-safe. (Tested with all the same unit tests I ran on the original Data instance - I just had to put in an adapter to catch RemoteExceptions and conver