OpenJPA
  1. OpenJPA
  2. OPENJPA-2257

Concurreny in org.apache.openjpa.persistence.EntityManagerImpl.getProperties leads to NullPointer and ConcurrentModificationException

    Details

      Description

      A call of EntityManager.getProperties() can lead to NullPointer and ConcurrentModificationException. Issue occurs right after start up of the overlying JEE application if multiple EntityManager instance are created at same time.

      Please find the issued stack trace below:

      Caused by: java.lang.NullPointerException
      at java.lang.String.compareTo(String.java:482)
      at java.lang.String.compareTo(String.java:31)
      at java.util.TreeMap.cmp(TreeMap.java:4514)
      at java.util.TreeMap.putImpl(TreeMap.java:4556)
      at java.util.TreeMap.put(TreeMap.java:4536)
      at java.util.TreeSet.add(TreeSet.java:122)
      at
      org.apache.openjpa.lib.conf.ConfigurationImpl.getPropertyKeys(ConfigurationImpl.java:708)
      at
      org.apache.openjpa.kernel.BrokerImpl.getSupportedProperties(BrokerImpl.java:729)
      at
      org.apache.openjpa.kernel.DelegatingBroker.getSupportedProperties(DelegatingBroker.java:223)
      at
      org.apache.openjpa.persistence.EntityManagerImpl.getProperties(EntityManagerImpl.java:1624)
      ... 33 more

      1. OpenJPABugTest.zip
        7.74 MB
        Stephan Hagedorn
      2. OPENJPA-2257.patch
        4 kB
        Albert Lee

        Activity

        Hide
        Heath Thomann added a comment -

        In addition to Albert's changes, I've found that more changes were necessary. That is, synchronizing on '_supportedKeys' in ConfigurationImpl.getPropertyKeys is necessary and fixes one ConcurrentModificationException/NPE, however, I found another spot which causes a ConcurrentModificationException when the returned '_supportedKeys' is added to by other areas of code. The exception and stack I saw was the following:

        java.util.ConcurrentModification
        at java.util.TreeMap$AbstractMapIterator.makeNext(TreeMap.java:5794)
        at java.util.TreeMap$UnboundedKeyIterator.next(TreeMap.java:5896)
        at org.apache.openjpa.persistence.EntityManagerImpl.getProperties(EntityManagerImpl.java:1624)
        at Test.run(Test.java:26)

        From the stack, EntityManagerImpl.getProperties is operating on the the set (i.e. '_supportedKeys') returned by ConfigurationImpl.getPropertyKeys. In a single threaded environment, operating on the set is just fine. However, in my test, I had found that if one thread is operating on the set (as shown in the stack above), yet another thread is adding to the set, the ConcurrentModification exception can occur. While the details of my tests are rather long and nearly impossible to hit in a real world situation, let me basically state that I found that if a thread was in the 'EntityManagerImpl.getProperties' listed above, and another thread was in this portion of code in BrokerImpl:

        public Set<String> getSupportedProperties() {
        Set<String> keys = _conf.getPropertyKeys();
        for (String s : _supportedPropertyNames)
        keys.add("openjpa." + s);

        A ConcurrentModification exception could occur given that the one thread operating in 'getSupportedProperties' could add to 'keys' (which is really the '_supportedKeys'), and another thread could be iterating over this same set. This issue seemed to be further complicated, and dependent on, the JDK in use. I created the above exception by using an IBM JDK which appears to add some inner classes (e.g. .TreeMap$UnboundedKeyIterator) to a TreeMap which performs some additional checks for concurrent access. Bottom line is that the fix appears to be to return a copy of '_supportedKeys' which is returned by ConfigurationImpl.getPropertyKeys.

        Show
        Heath Thomann added a comment - In addition to Albert's changes, I've found that more changes were necessary. That is, synchronizing on '_supportedKeys' in ConfigurationImpl.getPropertyKeys is necessary and fixes one ConcurrentModificationException/NPE, however, I found another spot which causes a ConcurrentModificationException when the returned '_supportedKeys' is added to by other areas of code. The exception and stack I saw was the following: java.util.ConcurrentModification at java.util.TreeMap$AbstractMapIterator.makeNext(TreeMap.java:5794) at java.util.TreeMap$UnboundedKeyIterator.next(TreeMap.java:5896) at org.apache.openjpa.persistence.EntityManagerImpl.getProperties(EntityManagerImpl.java:1624) at Test.run(Test.java:26) From the stack, EntityManagerImpl.getProperties is operating on the the set (i.e. '_supportedKeys') returned by ConfigurationImpl.getPropertyKeys. In a single threaded environment, operating on the set is just fine. However, in my test, I had found that if one thread is operating on the set (as shown in the stack above), yet another thread is adding to the set, the ConcurrentModification exception can occur. While the details of my tests are rather long and nearly impossible to hit in a real world situation, let me basically state that I found that if a thread was in the 'EntityManagerImpl.getProperties' listed above, and another thread was in this portion of code in BrokerImpl: public Set<String> getSupportedProperties() { Set<String> keys = _conf.getPropertyKeys(); for (String s : _supportedPropertyNames) keys.add("openjpa." + s); A ConcurrentModification exception could occur given that the one thread operating in 'getSupportedProperties' could add to 'keys' (which is really the '_supportedKeys'), and another thread could be iterating over this same set. This issue seemed to be further complicated, and dependent on, the JDK in use. I created the above exception by using an IBM JDK which appears to add some inner classes (e.g. .TreeMap$UnboundedKeyIterator) to a TreeMap which performs some additional checks for concurrent access. Bottom line is that the fix appears to be to return a copy of '_supportedKeys' which is returned by ConfigurationImpl.getPropertyKeys.
        Hide
        Albert Lee added a comment -

        Fixed only in trunk. To get fixes for 2.1.x, you'll need to request it from WebSphere service channel.

        Show
        Albert Lee added a comment - Fixed only in trunk. To get fixes for 2.1.x, you'll need to request it from WebSphere service channel.
        Hide
        Albert Lee added a comment -

        The problem is caused by the assumption that the configurationImpl obtained from emf is threadsafe since all the configuration values are frozen once the emf is created. However the broker getProperty() implementation defers the building up of the property key collection (_supportedKeys) in the configurationImpl when it is needed, but the _supportedKeys is not synchronized( thread safe), hence the observed exceptions.

        Show
        Albert Lee added a comment - The problem is caused by the assumption that the configurationImpl obtained from emf is threadsafe since all the configuration values are frozen once the emf is created. However the broker getProperty() implementation defers the building up of the property key collection (_supportedKeys) in the configurationImpl when it is needed, but the _supportedKeys is not synchronized( thread safe), hence the observed exceptions.

          People

          • Assignee:
            Albert Lee
            Reporter:
            Stephan Hagedorn
          • Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development