Thread Management and EJBs - Not Again !!!

Discussions

EJB design: Thread Management and EJBs - Not Again !!!

  1. Thread Management and EJBs - Not Again !!! (5 messages)

    I wish to put this topic to rest forever! Just when I convinced myself that synchronized keyword cannot be used in neither EJBs nor its helper classes (http://java.sun.com/blueprints/qanda/ejb_tier/restrictions.html) I came across a comment (http://www.mail-archive.com/ejb-interest at java dot sun dot com/msg24700.html) that read "It is only the EJB methods on which you should not 'synchronize'. You can write helper/utility classes on
    whose methods/classes you can synchronize."

    How far is this true?

    Also, in one of my interactions before (http://www.theserverside.com/discussions/thread.tss?thread_id=22414) Gal had mentioned that its ok to use synchronize locally as long as
    A) no calls are made to any of the resources on the app-server from these synchronized blocks
    B) no shared object is returned to the EJBs instances.

    Even though the use of synchronize keyword here violates the specs, yet its use within this limitation seems reasonable. However, the container will not be aware of such thread managements, so wont that lead to possible problems?

    Thanks
  2. The main reason for the "no synchronization" rule in the EJB specs is that the EJB server itself will be synchronizing access to the EJB objects. Since developers have no idea how this will be done, synchronization in the EJB class itself is risking thread deadlock.

    However, if you synchronize on classes that the EJB server itself can't access, the risk of deadlock can be significantly reduce (and eliminated if you are careful and know what you are doing). For example, synchronizing on private or package-access classes and objects that are used by your EJBs should be safe.

    Personally, I restrict my EJB-related synchronization to private objects used by a single EJB class, and I have yet to have a problem.
  3. Lets take an example here.

    Lets assume that there is a synchronized block in the helper class that takes a connection from the connection pool and populates a cache. The JDBC Connection Pool is managed by the container. When we use the "synchronised" keyword in the helper class we are managing the threads ourselves which the container is oblivious of. Hence when we try to access the connection pool from inside the synchronized block we are interfering with the container's ability to control its components' lifecycle. Probably this would be a good example of the "no synchronization" rule.

    Say if we have a method called check_Data_in_Map() that is not been synchronized and returns a boolean. There is a private hashtable that has been synchronized.

    private synchronized Hashtable object;

    boolean check_Data_in_Map() {// not synchronized to ensure we dont manage threads accessing the app-server.

     if Map does not contain data
     {
       get data from database
       add(data);
       return true;
     }
     else return true;
     
    }

    synchronized Add(String)
    {....Add Data to the hashtable.......}

    Will the above code snippet depict the scenario that you are talking about - about synchronizing private variable ?

    Are we not trying to sychronise the access of the ejb instances in the Add method ? Wouldnt this against the grain of the ejb spec ?
  4. I am not sure if I am following your example correctly, since the "synchronized" keywork cannot be applied to instance variables. Let me give you a different example. Here is a skeleton of a helper class I use for Primary Key generation:

    public class IdCounter {
        public synchronized long getNextId() {
           // Load values from the database using JDBC every 1024 requests
           // Otherwise, use in-memory cached values for better effeciency
        }

        // Other private methods and variables ...
    }

    In my EJB class:

    public class KeyGeneratorEJB implements SessionBean {
        private static final IdCounter idCounter = new IdCounter();

        public Long getNextId() {
            return new Long(idCounter.getNextId());
        }

        // Other methods ...
    }

    The IdCounter is effectively a singleton (static), and access to its only public method is synchronized. This is a double violation of the EJB spec, but has never caused me trouble. The reason why I believe this is safe is because I can't think of a way the system could deadlock.

    The simplest way to get a deadlock is to have thread A with a lock on object X try to use object Y at the same time thread B with a lock on object Y tries to use object X. More complex deadlocks generally occur because of cycles leading to the above.

    Since the IdCounter object is private to the KeyGeneratorEJB class, only instances of that class can lock the IdCounter. Furthermore, the IdCounter is the only synchronized object used directly by the KeyGeneratorEJB class, there should never be a situation where KeyGeneratorEJB object tries to get a lock on the IdCounter while holding a lock on something else.

    If you are super paranoid, you can limit the KeyGeneratorEJB to a pool size of 1, meaning that any thread having a lock on the IdCounter also has a lock on the KeyGeneratorEJB, making it impossible for any other thread to use the KeyGeneratorEJB. In practice, I have never found this to be necessary (though there is not much benefit to having more than one KeyGeneratorEJB anyway).

    I suppose there is some risk of locks on the JDBC related objects used by the IdCounter, but (a) the Connection object itself won't be synchronized because it will never be used by more than one thread and (b) the Connection pool itself must have good locking semantics, otherwise the EJBs themselves would deadlock when using the DataSource for the pool.

    For this case, I have decided that the benefits of in-memory caching of set of generated keys outweigh the risks and costs of synchronization.
  5. I truely appreciate your patience, Paul. Your comments have helped me enormously in having a deeper understanding of this issue. Thanks.
    To start with the "private synchronized List object;" was only meant to be symbolic representation of something like
    "private List authorizedList = Collections.synchronizedList(new ArrayList());" which I didnt bother about.

    >The IdCounter is effectively a singleton (static), and access to its only public method is synchronized. This is a double violation of the EJB spec< The IdCounter is legal since you qualified it with a final too. Specs mandate static variables to be final, which it is, in your case.

    The only issue here is with the synchronized keyword. And I too see how it can create a deadlock.

    I might have been a little confused when I earlier said that -
    "there is a synchronized block in the helper class that takes a connection from the connection pool and populates a cache. The JDBC Connection Pool is managed by the container. When we use the "synchronised" keyword in the helper class we are managing the threads ourselves which the container is oblivious of. Hence when we try to access the connection pool from inside the synchronized block we are interfering with the container's ability to control its components' lifecycle. Probably this would be a good example of the "no synchronization" rule" - Actually, Gal (http://www.theserverside.com/discussions/thread.tss?thread_id=22414) had spoken of the problem of management by container happening in the case of NEW THREADS, SPAWNED inside the ejb, trying to access the app-server. This is outside the context of the example we are dealing with.

    Related to my problem, we are required to have a cache that will contain a certain number of records. If the records are not found in the cache then we perform JDBC operations and retrieve the records from database and put them in the cache. This is quite similar to your IdCounter example. Wondering if there could be some other way of doing it say like employing entity bean ! But then we have to create that many number of entity beans depending on the number of rows and cache them all on the server to simulate our cache or may be I am wrong !! I dont see another way to employ a cache without using the "forbidden" synchronised keyword here !

    The reason the specs put a blanket ban on the synchronised keyword is to prevent "IMPROPER" use of it that could lead to thread deadlocks. If it is used correctly then there shouldnt be a problem, is my guess ;)!

    Any comments?

    Thanks again.
  6. I think we are communicating effectively now :)
    Related to my problem, we are required to have a cache that will contain a certain number of records. If the records are not found in the cache then we perform JDBC operations and retrieve the records from database and put them in the cache. This is quite similar to your IdCounter example. Wondering if there could be some other way of doing it say like employing entity bean!
    You should be able to use a technique similar to the one I used for my IdCounter.

    1) Put your Cache as a private static variable of a Session Bean. The thread management issues for session beans are must simpler than entity beans, so you reduce your risk of deadlock.

    2) Put the appropriate record lookup logic in you Cache object. Don't use any other EJB, but do use a JNDI DataSource for the JDBC lookup to take advantage of connection pooling.

    3) It should be safe to have multiple synchronized methods in your Cache object, so long as you have exactly one cache object. Remember that synchronization locks in the Java language are locks on objects, not methods.

    4) Write public methods in you Session Bean that delegate to the methods of your Cache singleton. Don't put the synchronized keyword on any of your Session Bean methods; rely on the synchronization on the Cache object for thread safety.

    You probably want to load test your Session Bean to be on the safe side, but odds are very good that this will work.
    The reason the specs put a blanket ban on the synchronised keyword is to prevent "IMPROPER" use of it that could lead to thread deadlocks. If it is used correctly then there shouldnt be a problem, is my guess ;)!
    That is my experience.