Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

Bug 319765

Summary:

Make Public EntityManagerFactoryProvider Collections Thread Safe

Product:

z_Archived

Reporter:

Shaun Smith <shaun.smith>

Component:

Eclipselink

Assignee:

Nobody - feel free to take it <nobody>

Status:

CLOSED FIXED

QA Contact:

Severity:

normal

Priority:

CC:

michael.f.obrien, tom.ware

Version:

unspecified

Target Milestone:

---

Hardware:

OS:

Windows XP

Whiteboard:

Bug Depends on:

Bug Blocks:

331108

Attachments:

Description	Flags
Patch	none
Suggested patch	none
patch - take 2	none

Description Shaun Smith

2010-07-13 13:52:11 EDT

The public static variables EntityManagerFactoryProvider.initialEmSetupImpls and EntityManagerFactoryProvider.initialPuInfos are both implemented as non-thread safe HashMap but the potential exists for concurrent modification.  These collections are created and assigned in JavaSECMPInitializer.initialize and remove() is called on initialEmSetupImpls in PersistenceProvider.createEntityManagerFactory.  In the Java SE and EE environments concurrency is not an issue.  However in OSGi concurrency is possible and rather than hope concurrency doesn't corrupt these lists code should be put in place to ensure they are not corrupted.

Ongoing work and learnings in Gemini may require this bug be "escalated".

Comment 1 Tom Ware

2010-08-09 13:53:36 EDT

Setting target and priority.  See the following page for details of the meanings of these fields:

http://wiki.eclipse.org/EclipseLink/Development/Bugs/Guidelines

Comment 2 Shaun Smith

2010-11-30 15:58:37 EST

Created attachment 184183 [details]
Patch

Resolves bug by initializing public maps statically to Hashtables.  Patch also removes conditional code with null checks since the maps are not never null.

Comment 3 Michael OBrien

2010-11-30 16:31:53 EST

>Shaun, patch looks good
I assume that Hashtable is being used instead of ConcurrentHashMap so that the read locking behavior of Hashtable assures us of synchronization - at the expense of a slight performance penalty.  I expect that the configurable concurrencyLevel of ConcurrentHashMap would not gaurantee contention since the number of users is unknown.
- the removed NPE checks are ok - we just need to test running an SE persistence unit in an EE container - see initialize()

Comment 4 Shaun Smith

2010-12-02 15:19:06 EST

Patch applied in SVN tx 8605.

Comment 5 Shaun Smith

2010-12-08 13:52:19 EST

Reopening to address concerns raised by Andrei.  Revised fix in progress.

Comment 6 Andrei Ilitchev

2010-12-09 17:05:39 EST

Created attachment 184907 [details]
Suggested patch

The patch associates JPAInitializers with their initializationClassloader. Initializer now can hold initialPuInfos and initialEmSetupImpls (those used to be statics).
Also initializer keeps a list of emSetupImpls that has at least one factory associated with them.

The main scenarious are:

SE with agent (no redeployment is possible). Only one initializer (corresponding to the system classloader) is created. No changes in functionality.

SE without agent. Redeployment is possible AND Eclipselink is not notified on undeployment. Therefore initialPuInfos and initialEmSetupImpls are not created (otherwise if no factory was ever created the may linger after redeployment forever). No changes in functionality - possible multiple initializers - one per class loader. As before the change the xml archive is read each time a new pu is created. Initializer is removed when no factories left.

OSGi. Redeployment is possible and controled. On undeploy loop through all emSetupImpls cached on the initializer corresponding to the classloader being undeployed and undeploy them all.

Comment 7 Andrei Ilitchev

2010-12-10 15:00:40 EST

Created attachment 184991 [details]
patch - take 2

Assorted fixes for concurrency and some exceptions handling.

Comment 8 Shaun Smith

2011-01-24 14:15:11 EST

Resolved by Bug 332743.

Comment 9 Eclipse Webmaster

2022-06-09 10:28:37 EDT

The Eclipselink project has moved to Github: https://github.com/eclipse-ee4j/eclipselink