Multithreading crash - ConcurrentModificationException evaluating subtyping relationship
See bug #4216. During investigation of this a multithreading problem was found. During type checking of a general comparison at GeneralComparison.java#293 we are doing
th.isSubtype(t1, NumericType.getInstance()) where t1 happens to be BuiltInAtomicType.STRING. This is leading to a
ConcurrentModificationException in TypeHierarchy.java#396 where we call
toSet(t2.getPlainMemberTypes()), iterating through the members of the union type
#1 Updated by Michael Kay over 2 years ago
- Status changed from New to In Progress
It seems that the synchronization in
NumericType.getPlainMemberTypes() isn't strong enough, though I'm not quite sure why.
It works if we make the synchronization stronger, but I think it's probably better to cut the complexity and return a new List each time, rather than trying to memoize it.
#2 Updated by Michael Kay over 2 years ago
Synchronising this method more strongly solves the immediate problem, but I'm now having considerable doubts about the idea that the
XPathEvaluator (and similarly the s9api
XPathCompiler) are really thread-safe, that is, allowing multiple XPath compilations to take place concurrently.
The problem is that both classes contain an
AbstractStaticContext, which contains a
KeyManager, and compiling an XPath expression can add key definitions to the
KeyManager without synchronization.
#3 Updated by Michael Kay over 2 years ago
The ownership and shareability of the KeyManager look pretty obscure in the code.
For XSLT, there needs to be one KeyManager per compiled package. To this end the KeyManager is held in the PackageData object (which for XSLT will always be a StylesheetPackage). The KeyManager also has a reference to its PackageData object, so it looks like a one-to-one relationship.
So one wonders why the
Executable also holds a
KeyManager, and why the
Controller gets it from the
Executable. It looks as if it's only the idref() function that uses the
KeyManager held in the Executable; and it looks as if it's only
StylesheetPackage that puts a
KeyManager in the
Executable. So it looks to me as if the
idref() function can go straight to
getController().getExecutable().getTopLevelPackage().getKeyManager(), and avoid holding the
KeyManager redundantly in the
QueryModule.getKeyManager() contains logic to ensure the
KeyManager is copied into the
packageData; we can eliminate that.
So this just leaves XPath; and the question is whether we want to continue ensuring that all XPath expressions compiled under a single XPathCompiler should share the same
KeyManager. If we do, we have to make it thread-safe. That's what I've chosen to do.
Please register to edit this issue