Bugzilla – Full Text Bug Listing
| Summary: | Need to improve multithread protection in EMF initialization | ||||||
|---|---|---|---|---|---|---|---|
| Product: | [WebTools] WTP Java EE Tools | Reporter: | Carl Anderson <ccc> | ||||
| Component: | jst.j2ee | Assignee: | Carl Anderson <ccc> | ||||
| Status: | RESOLVED FIXED | QA Contact: | Chuck Bridgham <cbridgha> | ||||
| Severity: | major | ||||||
| Priority: | P2 | CC: | david_williams, kaloyan, robban.hoglund | ||||
| Version: | 3.2.3 | Flags: | david_williams:
pmc_approved+
ccc: pmc_approved? (raghunathan.srinivasan) ccc: pmc_approved? (naci.dai) robban.hoglund: pmc_approved+ robban.hoglund: pmc_approved+ kaloyan: pmc_approved+ ccc: pmc_approved? (cbridgha) cbridgha: review+ |
||||
| Target Milestone: | 3.2.4 | ||||||
| Hardware: | PC | ||||||
| OS: | Windows XP | ||||||
| Whiteboard: | PMC_approved | ||||||
| Bug Depends on: | |||||||
| Bug Blocks: | 376255, 378237 | ||||||
| Attachments: |
|
||||||
The problem here is that two threads got into the body of the init() method, beyond the protection at the beginning (as per the following example):
public static JcaPackage init() {
if (isInited) return (JcaPackage)EPackage.Registry.INSTANCE.getEPackage(JcaPackage.eNS_URI);
// Obtain or create and register package
JcaPackageImpl theJcaPackage = (JcaPackageImpl)(EPackage.Registry.INSTANCE.get(eNS_URI) instanceof JcaPackageImpl ? EPackage.Registry.INSTANCE.get(eNS_URI) : new JcaPackageImpl());
isInited = true;
The question is, how do we reduce this window any further?
Created attachment 193942 [details]
Make isInited volatile
One possible improvement here is to make isInited volatile. This helps protect multithreaded/multicore accesses to this variable, since it could conceivably be cached in the various cores, and thus when one instance marks isInited as true, the other could be reading the (improperly cached) false value, and then proceed.
approved Committed to HEAD for WTP 3.3 M7 Explain why you believe this is a stop-ship defect. Or, if it is a "hotbug" (requested by an adopter) please document it as such. When this exception occurs, the EMF model classes cannot be loaded/used. (The JVM marks them as having errors, and will not try to initialize them again.) Is there a work-around? If so, why do you believe the work-around is insufficient? No workaround. How has the fix been tested? Is there a test case attached to the bugzilla record? Has a JUnit Test been added? The Java EE JUnit bucket has been run against this fix. Give a brief technical overview. Who has reviewed this fix? Chuck Bridgham has reviewed this fix. By marking these static booleans as volatile, we are insuring that the value for isInited is consistent across threads/cores, such that as soon as one thread initializes the instance, no other thread will do so. What is the risk associated with this fix? This is an extremely low risk fix. Committed to R3_2_maintenance for WTP 3.2.4 Sorry, The fix does not work. I have this problem and I have analyzed you fix and see some flaws.
In the method initalizePackageContents you have the following construction:
boolean hasLock = false;
try {
hasLock = J2EEInit.aquireInitializePackageContentsLock();
} catch (InterruptedException e) {
J2EECorePlugin.logError(e);
}
The problem is that you never read the hasLock variable and the code will pass the barrier and perform multiple initializations, and therefore the concurrancy problem is still a potential threat.
private volatile static boolean isInited = false; And relying on the construction above do not solve the multithreading problem because of the fact that multiple threads may pass before it isInited is set to true and multiple initializations will perform concurrently. |
An adopter product recently encountered the following exceptions: Exception in thread "Thread-7" java.lang.ExceptionInInitializerError at java.lang.J9VMInternals.initialize(J9VMInternals.java:222) at sun.misc.Unsafe.ensureClassInitialized(Native Method) at sun.reflect.UnsafeFieldAccessorFactory.newFieldAccessor(UnsafeFieldAccessorFactory.java:37) at sun.reflect.ReflectionFactory.newFieldAccessor(ReflectionFactory.java:134) at java.lang.reflect.Field.acquireFieldAccessor(Field.java:945) at java.lang.reflect.Field.getFieldAccessor(Field.java:912) at java.lang.reflect.Field.get(Field.java:371) at org.eclipse.emf.ecore.plugin.RegistryReader$EPackageDescriptor.getEPackage(RegistryReader.java:274) at org.eclipse.emf.ecore.impl.EPackageRegistryImpl.getEPackage(EPackageRegistryImpl.java:133) at org.eclipse.jst.j2ee.internal.J2EEInit$20.run(J2EEInit.java:351) at java.lang.Thread.run(Thread.java:736) Caused by: java.lang.NullPointerException at org.eclipse.emf.common.util.AbstractEList.getNonDuplicates(AbstractEList.java:1167) at org.eclipse.emf.common.util.AbstractEList.addAll(AbstractEList.java:372) at org.eclipse.emf.ecore.impl.EClassImpl.getEAllOperations(EClassImpl.java:924) at org.eclipse.emf.ecore.impl.EClassImpl.freeze(EClassImpl.java:131) at org.eclipse.emf.ecore.impl.EModelElementImpl.freeze(EModelElementImpl.java:100) at org.eclipse.emf.ecore.impl.EPackageImpl.freeze(EPackageImpl.java:203) at org.eclipse.jst.j2ee.jca.internal.impl.JcaPackageImpl.init(JcaPackageImpl.java:196) at org.eclipse.jst.j2ee.jca.JcaPackage.<clinit>(JcaPackage.java:276) at java.lang.J9VMInternals.initializeImpl(Native Method) at java.lang.J9VMInternals.initialize(J9VMInternals.java:200) ... 10 more Exception in thread "Thread-6" java.lang.ExceptionInInitializerError at java.lang.J9VMInternals.initialize(J9VMInternals.java:222) at sun.misc.Unsafe.ensureClassInitialized(Native Method) at sun.reflect.UnsafeFieldAccessorFactory.newFieldAccessor(UnsafeFieldAccessorFactory.java:37) at sun.reflect.ReflectionFactory.newFieldAccessor(ReflectionFactory.java:134) at java.lang.reflect.Field.acquireFieldAccessor(Field.java:945) at java.lang.reflect.Field.getFieldAccessor(Field.java:912) at java.lang.reflect.Field.get(Field.java:371) at org.eclipse.emf.ecore.plugin.RegistryReader$EPackageDescriptor.getEPackage(RegistryReader.java:274) at org.eclipse.emf.ecore.impl.EPackageRegistryImpl.getEPackage(EPackageRegistryImpl.java:133) at org.eclipse.jst.j2ee.internal.J2EEInit$20.run(J2EEInit.java:351) at java.lang.Thread.run(Thread.java:736) Caused by: java.lang.NullPointerException at org.eclipse.emf.common.util.BasicEList.contains(BasicEList.java:172) at org.eclipse.emf.common.util.AbstractEList.add(AbstractEList.java:301) at org.eclipse.emf.ecore.impl.ESuperAdapter.notifyChanged(ESuperAdapter.java:139) at org.eclipse.emf.common.notify.impl.BasicNotifierImpl.eNotify(BasicNotifierImpl.java:380) at org.eclipse.emf.common.notify.impl.NotificationImpl.dispatch(NotificationImpl.java:1033) at org.eclipse.emf.common.notify.impl.NotifyingListImpl.addUnique(NotifyingListImpl.java:367) at org.eclipse.emf.common.util.AbstractEList.add(AbstractEList.java:345) at org.eclipse.emf.ecore.impl.EClassImpl$9.delegateAdd(EClassImpl.java:1698) at org.eclipse.emf.ecore.impl.EClassImpl$9.delegateAdd(EClassImpl.java:1) at org.eclipse.emf.common.util.DelegatingEList.addUnique(DelegatingEList.java:340) at org.eclipse.emf.common.notify.impl.DelegatingNotifyingListImpl.doAddUnique(DelegatingNotifyingListImpl.java:385) at org.eclipse.emf.common.notify.impl.DelegatingNotifyingListImpl.addUnique(DelegatingNotifyingListImpl.java:281) at org.eclipse.emf.common.util.AbstractEList.add(AbstractEList.java:307) at org.eclipse.jst.j2ee.jca.internal.impl.JcaPackageImpl.initializePackageContents(JcaPackageImpl.java:949) at org.eclipse.jst.j2ee.jca.internal.impl.JcaPackageImpl.init(JcaPackageImpl.java:193) at org.eclipse.jst.j2ee.jca.JcaPackage.<clinit>(JcaPackage.java:276) at java.lang.J9VMInternals.initializeImpl(Native Method) at java.lang.J9VMInternals.initialize(J9VMInternals.java:200) ... 10 more