merks
commented
1 year ago Sorry, I overlooked this issue being opened. The implementation is intentionally allowed to have race conditions. The logic in org.eclipse.emf.ecore.impl.EClassImpl.getEAllStructuralFeatures().EAllStructuralFeaturesList.init() builds new lists and all the adds to those lists are objects that cannot be null. At the end, it shrinks these lists and finally assignment their underlying array to the containments/crossReferences fields. It's inexplicable to me how that array can contain null even if 10 threads are doing this same thing at once. Perhaps the containments/crossReferences fields should be volatile. Only via some drastic reordering could the final array not have been properly initialized by the preceding shrink() call.
Another possibility to avoid any problem, regardless of the cause would be to do something like this in org.eclipse.emf.ecore.impl.EClassImpl.freeze()
@Override
protected void freeze()
{
getEAllAttributes();
getEAllReferences();
getEAllContainments();
getEAllOperations();
((FeatureSubsetSupplier)getEAllStructuralFeatures()).containments(); // Force containments/crossReferences to be initialized.
getEAllSuperTypes();
getEAllGenericSuperTypes();
getESuperAdapter().getSubclasses().clear();
if (eStructuralFeatures != null)
{
for (int i = 0, size = eStructuralFeatures.size(); i < size; ++i)
{
freeze(eStructuralFeatures.get(i));
}
}
if (eOperations != null)
{
for (int i = 0, size = eOperations.size(); i < size; ++i)
{
freeze(eOperations.get(i));
}
}
// Bug 433108: Lock in the shared extended metadata for this class
ExtendedMetaData.INSTANCE.getName(this);
super.freeze();
}
gireeshpunathil
A user reported NPE in org.eclipse.emf.ecore_2.10.2.v20150123-0348.jar.
From the user: I have system dump generated from this NPE. Analysis of that system dump only seems to confirm my working hypothesis, which is that this NPE is due to lack of thread safety of org.eclipse.emf.ecore.impl.EAllStructuralFeaturesList.containments(). I understood that lazily intialization was not thread-safe, but once done, objects could be safely read by multiple threads. Some instances of EClassImpl in v2.10.2 are mutated by all reads of containments(). The returned EStructuralFeature[] is non-null, but its first and only element is null, due to lack of synchronization. FeatureIteratorImpl.hasNext is not designed to tolerate null elements in such arrays. (In the system dump, the array is non-null, and the first element is also non-null. I wouldn't expect a system dump to reflect these transient effects.)
Does this seem like a sane explanation for the NPE? Is there a better explanation? (I can share additional details if necessary.) We can possibly patch our application to fix this thread safey problem (by adopting the change from https://bugs.eclipse.org/bugs/show_bug.cgi?id=461398, which avoids the subsequent mutations), but idk that the NPE is caused by lack of thread safety.
The NPE has only been seen on AIX/PowerPC (ppc64). We have tried to reproduce on x86_64 (16 core) and AIX/Power8 (8 core) without success.
Any suggestions on isolation, root cause analysis are much appreciated!