Bug #4218
closedMultithreading crash - ConcurrentModificationException evaluating subtyping relationship
100%
Description
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 :numeric
.
Related issues
Updated by Michael Kay over 5 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.
Updated by Michael Kay over 5 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.
Updated by Michael Kay over 5 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 Executable
.
QueryModule.getKeyManager()
contains logic to ensure the Executable
's 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.
Updated by Michael Kay over 5 years ago
- Status changed from In Progress to Resolved
- Fix Committed on Branch 9.9, trunk added
I believe this has been resolved.
Updated by O'Neil Delpratt over 5 years ago
- Status changed from Resolved to Closed
- % Done changed from 0 to 100
- Fixed in Maintenance Release 9.9.1.4 added
Bug fix applied in the Saxon 9.9.1.4 maintenance release
Updated by Michael Kay over 1 year ago
- Has duplicate Bug #6154: ConcurrentModificationException in net.sf.saxon.s9api.XsltCompiler.compile added
Please register to edit this issue