You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@openjpa.apache.org by "Kevin Sutter (JIRA)" <ji...@apache.org> on 2007/11/08 15:23:50 UTC

[jira] Created: (OPENJPA-437) EntityManagerFactory is not thread-safe

EntityManagerFactory is not thread-safe
---------------------------------------

                 Key: OPENJPA-437
                 URL: https://issues.apache.org/jira/browse/OPENJPA-437
             Project: OpenJPA
          Issue Type: Bug
          Components: kernel
    Affects Versions: 1.0.0, 1.0.1, 1.0.2, 1.1.0
            Reporter: Kevin Sutter
            Assignee: Kevin Sutter
             Fix For: 1.0.2, 1.1.0


Under certain conditions, we have discovered that the EntityManagerFactory is not thread safe when creating EntityManagers.  The problem is in the loadPersistentTypes method of the AbstractBrokerFactory.  There is an unprotected data structure (_pcClassNames) that can various problems (NullPointerException, IndexOutOfBoundsException, etc) when attempting to add new elements to the ArrayList.  Other similar datastructures in this part are properly synchronized (_pcClassLoaders for example), but somehow we missed this one.

A common scenario where this might be encountered is if your SLSB has an injected PersistenceUnit (EntityManagerFactory), but is attempting to create the EntityManager during a post-bean creation method (@PostConstruct).  In this case, the SLSB instances are probably using the same EMF instance (of course this would depend on your application server implementation).  If you have this type of environment, then hitting these SLSB's with multiple clients could get you into this situation.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Closed: (OPENJPA-437) EntityManagerFactory is not thread-safe

Posted by "Kevin Sutter (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/OPENJPA-437?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Kevin Sutter closed OPENJPA-437.
--------------------------------


> EntityManagerFactory is not thread-safe
> ---------------------------------------
>
>                 Key: OPENJPA-437
>                 URL: https://issues.apache.org/jira/browse/OPENJPA-437
>             Project: OpenJPA
>          Issue Type: Bug
>          Components: kernel
>    Affects Versions: 1.0.0, 1.0.1, 1.0.2, 1.1.0
>            Reporter: Kevin Sutter
>            Assignee: Kevin Sutter
>             Fix For: 1.0.2, 1.1.0
>
>         Attachments: OPENJPA-437-nosync.patch
>
>
> Under certain conditions, we have discovered that the EntityManagerFactory is not thread safe when creating EntityManagers.  The problem is in the loadPersistentTypes method of the AbstractBrokerFactory.  There is an unprotected data structure (_pcClassNames) that can various problems (NullPointerException, IndexOutOfBoundsException, etc) when attempting to add new elements to the ArrayList.  Other similar datastructures in this part are properly synchronized (_pcClassLoaders for example), but somehow we missed this one.
> A common scenario where this might be encountered is if your SLSB has an injected PersistenceUnit (EntityManagerFactory), but is attempting to create the EntityManager during a post-bean creation method (@PostConstruct).  In this case, the SLSB instances are probably using the same EMF instance (of course this would depend on your application server implementation).  If you have this type of environment, then hitting these SLSB's with multiple clients could get you into this situation.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Reopened: (OPENJPA-437) EntityManagerFactory is not thread-safe

Posted by "Patrick Linskey (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/OPENJPA-437?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Patrick Linskey reopened OPENJPA-437:
-------------------------------------


Reopening pending discussion of synchronization.

> EntityManagerFactory is not thread-safe
> ---------------------------------------
>
>                 Key: OPENJPA-437
>                 URL: https://issues.apache.org/jira/browse/OPENJPA-437
>             Project: OpenJPA
>          Issue Type: Bug
>          Components: kernel
>    Affects Versions: 1.0.0, 1.0.1, 1.0.2, 1.1.0
>            Reporter: Kevin Sutter
>            Assignee: Kevin Sutter
>             Fix For: 1.0.2, 1.1.0
>
>         Attachments: OPENJPA-437-nosync.patch
>
>
> Under certain conditions, we have discovered that the EntityManagerFactory is not thread safe when creating EntityManagers.  The problem is in the loadPersistentTypes method of the AbstractBrokerFactory.  There is an unprotected data structure (_pcClassNames) that can various problems (NullPointerException, IndexOutOfBoundsException, etc) when attempting to add new elements to the ArrayList.  Other similar datastructures in this part are properly synchronized (_pcClassLoaders for example), but somehow we missed this one.
> A common scenario where this might be encountered is if your SLSB has an injected PersistenceUnit (EntityManagerFactory), but is attempting to create the EntityManager during a post-bean creation method (@PostConstruct).  In this case, the SLSB instances are probably using the same EMF instance (of course this would depend on your application server implementation).  If you have this type of environment, then hitting these SLSB's with multiple clients could get you into this situation.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (OPENJPA-437) EntityManagerFactory is not thread-safe

Posted by "Patrick Linskey (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/OPENJPA-437?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12544156 ] 

Patrick Linskey commented on OPENJPA-437:
-----------------------------------------

Make that "a partial fix". OPENJPA-449 only deals with _pcClassLoaders.

> EntityManagerFactory is not thread-safe
> ---------------------------------------
>
>                 Key: OPENJPA-437
>                 URL: https://issues.apache.org/jira/browse/OPENJPA-437
>             Project: OpenJPA
>          Issue Type: Bug
>          Components: kernel
>    Affects Versions: 1.0.0, 1.0.1, 1.0.2, 1.1.0
>            Reporter: Kevin Sutter
>            Assignee: Kevin Sutter
>             Fix For: 1.0.2, 1.1.0
>
>
> Under certain conditions, we have discovered that the EntityManagerFactory is not thread safe when creating EntityManagers.  The problem is in the loadPersistentTypes method of the AbstractBrokerFactory.  There is an unprotected data structure (_pcClassNames) that can various problems (NullPointerException, IndexOutOfBoundsException, etc) when attempting to add new elements to the ArrayList.  Other similar datastructures in this part are properly synchronized (_pcClassLoaders for example), but somehow we missed this one.
> A common scenario where this might be encountered is if your SLSB has an injected PersistenceUnit (EntityManagerFactory), but is attempting to create the EntityManager during a post-bean creation method (@PostConstruct).  In this case, the SLSB instances are probably using the same EMF instance (of course this would depend on your application server implementation).  If you have this type of environment, then hitting these SLSB's with multiple clients could get you into this situation.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Updated: (OPENJPA-437) EntityManagerFactory is not thread-safe

Posted by "Patrick Linskey (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/OPENJPA-437?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Patrick Linskey updated OPENJPA-437:
------------------------------------

    Attachment: OPENJPA-437-nosync.patch

I'm a bit concerned about adding the synchronization. While this addresses the thread safety issue, it also introduces a bottleneck for applications with a large number of concurrent threads creating new EntityManagers. This was not a bottleneck in the previous (unsafe) code since the synchronized repository call was only made once.

Attached is a patch that I believe is both thread-safe and unsynchronized. There might be a better datastructure other than a ConcurrentReferenceHashSet, but I think that it should do the job.

> EntityManagerFactory is not thread-safe
> ---------------------------------------
>
>                 Key: OPENJPA-437
>                 URL: https://issues.apache.org/jira/browse/OPENJPA-437
>             Project: OpenJPA
>          Issue Type: Bug
>          Components: kernel
>    Affects Versions: 1.0.0, 1.0.1, 1.0.2, 1.1.0
>            Reporter: Kevin Sutter
>            Assignee: Kevin Sutter
>             Fix For: 1.0.2, 1.1.0
>
>         Attachments: OPENJPA-437-nosync.patch
>
>
> Under certain conditions, we have discovered that the EntityManagerFactory is not thread safe when creating EntityManagers.  The problem is in the loadPersistentTypes method of the AbstractBrokerFactory.  There is an unprotected data structure (_pcClassNames) that can various problems (NullPointerException, IndexOutOfBoundsException, etc) when attempting to add new elements to the ArrayList.  Other similar datastructures in this part are properly synchronized (_pcClassLoaders for example), but somehow we missed this one.
> A common scenario where this might be encountered is if your SLSB has an injected PersistenceUnit (EntityManagerFactory), but is attempting to create the EntityManager during a post-bean creation method (@PostConstruct).  In this case, the SLSB instances are probably using the same EMF instance (of course this would depend on your application server implementation).  If you have this type of environment, then hitting these SLSB's with multiple clients could get you into this situation.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (OPENJPA-437) EntityManagerFactory is not thread-safe

Posted by "Patrick Linskey (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/OPENJPA-437?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12562728#action_12562728 ] 

Patrick Linskey commented on OPENJPA-437:
-----------------------------------------

In an offline discussion with Kevin, I realized that _pcClassNames is now read-only, so I think we should be able to replace it with a regular HashSet instead of a ConcurrentReferenceHashSet. Does that sound correct?

> EntityManagerFactory is not thread-safe
> ---------------------------------------
>
>                 Key: OPENJPA-437
>                 URL: https://issues.apache.org/jira/browse/OPENJPA-437
>             Project: OpenJPA
>          Issue Type: Bug
>          Components: kernel
>    Affects Versions: 1.0.0, 1.0.1, 1.0.2, 1.1.0
>            Reporter: Kevin Sutter
>            Assignee: Kevin Sutter
>             Fix For: 1.0.2, 1.1.0
>
>         Attachments: OPENJPA-437-nosync.patch
>
>
> Under certain conditions, we have discovered that the EntityManagerFactory is not thread safe when creating EntityManagers.  The problem is in the loadPersistentTypes method of the AbstractBrokerFactory.  There is an unprotected data structure (_pcClassNames) that can various problems (NullPointerException, IndexOutOfBoundsException, etc) when attempting to add new elements to the ArrayList.  Other similar datastructures in this part are properly synchronized (_pcClassLoaders for example), but somehow we missed this one.
> A common scenario where this might be encountered is if your SLSB has an injected PersistenceUnit (EntityManagerFactory), but is attempting to create the EntityManager during a post-bean creation method (@PostConstruct).  In this case, the SLSB instances are probably using the same EMF instance (of course this would depend on your application server implementation).  If you have this type of environment, then hitting these SLSB's with multiple clients could get you into this situation.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (OPENJPA-437) EntityManagerFactory is not thread-safe

Posted by "Kevin Sutter (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/OPENJPA-437?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12559564#action_12559564 ] 

Kevin Sutter commented on OPENJPA-437:
--------------------------------------

Looking at just synchronizing the AbstractBrokerFactory.loadPersistentTypes method to resolve this problem.  As I continued to look at this problem, both _pcClassNames and _pcClassLoaders are not synchronized.  Patrick's fix for OPENJPA-449 to move the creation of the _pcClassLoaders structure to the constructor closed the window for the scenario described in OPENJPA-449, but it's still not protected from multiple client/thread access.  (Note, my previous comments in the Description about _pcClassLoaders being protected were not accurate.)

I could change both of these Collections to be synchronized, but the code was getting a bit ugly due to the still required synchronization for the iterators.  Since these two Collections are only modified in this method, an easy solution would be to make the method Synchronized.  Since this method is only used when creating new brokers, this shouldn't affect the through put.

Also, the logic for this method is based on the return value of the loadPersistentTypes on the MetaDataRepository instance.  This method is also synchronized, so we'd be following suit.

Any concerns about going the route of synchronizing the AbstractBrokerFactory.loadPersistentTypes method?

Kevin

> EntityManagerFactory is not thread-safe
> ---------------------------------------
>
>                 Key: OPENJPA-437
>                 URL: https://issues.apache.org/jira/browse/OPENJPA-437
>             Project: OpenJPA
>          Issue Type: Bug
>          Components: kernel
>    Affects Versions: 1.0.0, 1.0.1, 1.0.2, 1.1.0
>            Reporter: Kevin Sutter
>            Assignee: Kevin Sutter
>             Fix For: 1.0.2, 1.1.0
>
>
> Under certain conditions, we have discovered that the EntityManagerFactory is not thread safe when creating EntityManagers.  The problem is in the loadPersistentTypes method of the AbstractBrokerFactory.  There is an unprotected data structure (_pcClassNames) that can various problems (NullPointerException, IndexOutOfBoundsException, etc) when attempting to add new elements to the ArrayList.  Other similar datastructures in this part are properly synchronized (_pcClassLoaders for example), but somehow we missed this one.
> A common scenario where this might be encountered is if your SLSB has an injected PersistenceUnit (EntityManagerFactory), but is attempting to create the EntityManager during a post-bean creation method (@PostConstruct).  In this case, the SLSB instances are probably using the same EMF instance (of course this would depend on your application server implementation).  If you have this type of environment, then hitting these SLSB's with multiple clients could get you into this situation.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Resolved: (OPENJPA-437) EntityManagerFactory is not thread-safe

Posted by "Patrick Linskey (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/OPENJPA-437?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Patrick Linskey resolved OPENJPA-437.
-------------------------------------

    Resolution: Fixed

Good points. I resolved both with my most recent commit on this issue.

> EntityManagerFactory is not thread-safe
> ---------------------------------------
>
>                 Key: OPENJPA-437
>                 URL: https://issues.apache.org/jira/browse/OPENJPA-437
>             Project: OpenJPA
>          Issue Type: Bug
>          Components: kernel
>    Affects Versions: 1.0.0, 1.0.1, 1.0.2, 1.1.0
>            Reporter: Kevin Sutter
>            Assignee: Kevin Sutter
>             Fix For: 1.0.2, 1.1.0
>
>         Attachments: OPENJPA-437-nosync.patch
>
>
> Under certain conditions, we have discovered that the EntityManagerFactory is not thread safe when creating EntityManagers.  The problem is in the loadPersistentTypes method of the AbstractBrokerFactory.  There is an unprotected data structure (_pcClassNames) that can various problems (NullPointerException, IndexOutOfBoundsException, etc) when attempting to add new elements to the ArrayList.  Other similar datastructures in this part are properly synchronized (_pcClassLoaders for example), but somehow we missed this one.
> A common scenario where this might be encountered is if your SLSB has an injected PersistenceUnit (EntityManagerFactory), but is attempting to create the EntityManager during a post-bean creation method (@PostConstruct).  In this case, the SLSB instances are probably using the same EMF instance (of course this would depend on your application server implementation).  If you have this type of environment, then hitting these SLSB's with multiple clients could get you into this situation.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (OPENJPA-437) EntityManagerFactory is not thread-safe

Posted by "Kevin Sutter (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/OPENJPA-437?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12562686#action_12562686 ] 

Kevin Sutter commented on OPENJPA-437:
--------------------------------------

Patrick,
Thanks for taking a look at this.

I went through the same type of concerns when I was coming up with the original patch.  But, it was my understanding that the ConcurrentReferenceHashSet still has an unprotected Iterator.  So, later on in this same loadPersistentTypes() method, the _pcClassNames structure is iterated through.  To make this iterator safe, then I would have to synchronize around this as well.  Also, the _pcClassLoaders should also be made thread-safe since we're updating that structure in this same method.  When I put in all of the necessary safeguards, it looked like it might be cleaner just by making the method synchronized.

But, if we're hitting a bottleneck by making that method synchronized, then we probably need to change it.  I had not discovered that bottleneck yet...

So, do we need to add the following items to your patch?

o  Initialize _pcClassLoaders with a ConcurrentReferenceHashSet.
o  Protect the _pcClassNames iterator usage with a synch block

Thanks,
Kevin

> EntityManagerFactory is not thread-safe
> ---------------------------------------
>
>                 Key: OPENJPA-437
>                 URL: https://issues.apache.org/jira/browse/OPENJPA-437
>             Project: OpenJPA
>          Issue Type: Bug
>          Components: kernel
>    Affects Versions: 1.0.0, 1.0.1, 1.0.2, 1.1.0
>            Reporter: Kevin Sutter
>            Assignee: Kevin Sutter
>             Fix For: 1.0.2, 1.1.0
>
>         Attachments: OPENJPA-437-nosync.patch
>
>
> Under certain conditions, we have discovered that the EntityManagerFactory is not thread safe when creating EntityManagers.  The problem is in the loadPersistentTypes method of the AbstractBrokerFactory.  There is an unprotected data structure (_pcClassNames) that can various problems (NullPointerException, IndexOutOfBoundsException, etc) when attempting to add new elements to the ArrayList.  Other similar datastructures in this part are properly synchronized (_pcClassLoaders for example), but somehow we missed this one.
> A common scenario where this might be encountered is if your SLSB has an injected PersistenceUnit (EntityManagerFactory), but is attempting to create the EntityManager during a post-bean creation method (@PostConstruct).  In this case, the SLSB instances are probably using the same EMF instance (of course this would depend on your application server implementation).  If you have this type of environment, then hitting these SLSB's with multiple clients could get you into this situation.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Resolved: (OPENJPA-437) EntityManagerFactory is not thread-safe

Posted by "Kevin Sutter (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/OPENJPA-437?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Kevin Sutter resolved OPENJPA-437.
----------------------------------

    Resolution: Fixed

Resolved in both 1.0.x and 1.1.0 branches (svn 612846 revision).

> EntityManagerFactory is not thread-safe
> ---------------------------------------
>
>                 Key: OPENJPA-437
>                 URL: https://issues.apache.org/jira/browse/OPENJPA-437
>             Project: OpenJPA
>          Issue Type: Bug
>          Components: kernel
>    Affects Versions: 1.0.0, 1.0.1, 1.0.2, 1.1.0
>            Reporter: Kevin Sutter
>            Assignee: Kevin Sutter
>             Fix For: 1.0.2, 1.1.0
>
>
> Under certain conditions, we have discovered that the EntityManagerFactory is not thread safe when creating EntityManagers.  The problem is in the loadPersistentTypes method of the AbstractBrokerFactory.  There is an unprotected data structure (_pcClassNames) that can various problems (NullPointerException, IndexOutOfBoundsException, etc) when attempting to add new elements to the ArrayList.  Other similar datastructures in this part are properly synchronized (_pcClassLoaders for example), but somehow we missed this one.
> A common scenario where this might be encountered is if your SLSB has an injected PersistenceUnit (EntityManagerFactory), but is attempting to create the EntityManager during a post-bean creation method (@PostConstruct).  In this case, the SLSB instances are probably using the same EMF instance (of course this would depend on your application server implementation).  If you have this type of environment, then hitting these SLSB's with multiple clients could get you into this situation.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.