Race condition in TypeMapper.getSafeTypeByName

[Ostrzyciel]

Version

5.2.0

What happened?

I've been chasing down the reasons for a flaky test in (you guessed it) a scalatest suite. The test was failing randomly and seemingly for no reason on isomorphism checks of two graphs that should be identical.

After some debugging, I've discovered the following:

• The root cause is comparing literals with non-standard datatypes (i.e., datatypes not preloaded on Jena startup).
• The literal comparison returns false for identical literals because the datatype comparison returns false: https://github.com/apache/jena/blob/04f377d8aefc0cb8faeb62022d402ef17a6e6e96/jena-core/src/main/java/org/apache/jena/graph/impl/LiteralLabel.java#L477-L479
• LiteralLabel assumes that the datatypes can be compared by reference, which makes sense, because Jena maintains a global hashmap of all used datatypes in TypeMapper.
• From the debugger I got the info that somehow there were two instances of BaseDatatype with the same IRI on the heap (http://www.opengis.net/ont/geosparql#wktLiteral):
• Only one of these instances was registered in TypeMapper.uriToDT. I am constructing new datatypes only via the NodeFactory, which ALWAYS goes through the TypeMapper, so the other instance must have been overridden at some point. There is no other way it could have ended up on the heap.

I'm 99% sure that the issue is this part:

https://github.com/apache/jena/blob/04f377d8aefc0cb8faeb62022d402ef17a6e6e96/jena-core/src/main/java/org/apache/jena/datatypes/TypeMapper.java#L124-L140

If the datatype is not in the concurrent map, this is called:

https://github.com/apache/jena/blob/04f377d8aefc0cb8faeb62022d402ef17a6e6e96/jena-core/src/main/java/org/apache/jena/datatypes/TypeMapper.java#L187-L193

The call to uriToDT.put() will override the previous value in the map, if it exists. This is absolutely possible, because between the call to get() and put() we have no synchronization, so another thread may add the same datatype to the map in the meantime.

I don't have a reproducible example, but I think this is a pretty clear case of a race condition... also relatively easy to fix, I think. I'll try doing that.

Relevant output and stacktrace

No response

Are you interested in making a pull request?

Yes

[Ostrzyciel]

On a related note: calling this method can potentially wreak havoc: https://github.com/apache/jena/blob/04f377d8aefc0cb8faeb62022d402ef17a6e6e96/jena-core/src/main/java/org/apache/jena/datatypes/TypeMapper.java#L198

I think that it should have (at least) a warning that calling it may break literal comparisons, if there are still old literal instances on the heap somewhere.