Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ConcurrentModificationException in TypeInfoSetImpl#getXmlNs #1726

Open
schabe77 opened this issue Jul 13, 2023 · 6 comments
Open

ConcurrentModificationException in TypeInfoSetImpl#getXmlNs #1726

schabe77 opened this issue Jul 13, 2023 · 6 comments

Comments

@schabe77
Copy link

Hi,

I use the BingAds-API that uses CXF that uses jaxb-runtime-4.0.3 and just faced a ConcurrentModificationException.

Unfortunately I don't know what Bing and CXF do in detail, but maybe this stack trace helps:

java.util.ConcurrentModificationException: null
	at java.base/java.util.HashMap.computeIfAbsent(HashMap.java:1221)
	at org.glassfish.jaxb.runtime.v2.model.impl.TypeInfoSetImpl.getXmlNs(TypeInfoSetImpl.java:298)
	at org.glassfish.jaxb.runtime.v2.schemagen.XmlSchemaGenerator$Namespace.writeTo(XmlSchemaGenerator.java:565)
	at org.glassfish.jaxb.runtime.v2.schemagen.XmlSchemaGenerator.write(XmlSchemaGenerator.java:415)
	at org.glassfish.jaxb.runtime.v2.runtime.JAXBContextImpl.generateSchema(JAXBContextImpl.java:770)
	at org.apache.cxf.common.jaxb.JAXBUtils.generateJaxbSchemas(JAXBUtils.java:810)
	at org.apache.cxf.jaxb.JAXBDataBinding.generateJaxbSchemas(JAXBDataBinding.java:474)
	at org.apache.cxf.jaxb.JAXBDataBinding.initialize(JAXBDataBinding.java:391)
	at org.apache.cxf.service.factory.AbstractServiceFactoryBean.initializeDataBindings(AbstractServiceFactoryBean.java:87)
	at org.apache.cxf.wsdl.service.factory.ReflectionServiceFactoryBean.buildServiceFromClass(ReflectionServiceFactoryBean.java:469)
	at org.apache.cxf.jaxws.support.JaxWsServiceFactoryBean.buildServiceFromClass(JaxWsServiceFactoryBean.java:693)
	at org.apache.cxf.wsdl.service.factory.ReflectionServiceFactoryBean.initializeServiceModel(ReflectionServiceFactoryBean.java:529)
	at org.apache.cxf.wsdl.service.factory.ReflectionServiceFactoryBean.create(ReflectionServiceFactoryBean.java:262)
	at org.apache.cxf.jaxws.support.JaxWsServiceFactoryBean.create(JaxWsServiceFactoryBean.java:199)
	at org.apache.cxf.jaxws.ServiceImpl.createPort(ServiceImpl.java:466)
	at org.apache.cxf.jaxws.ServiceImpl.getPort(ServiceImpl.java:342)
	at org.apache.cxf.jaxws.ServiceImpl.getPort(ServiceImpl.java:337)
	at jakarta.xml.ws.Service.getPort(Service.java:210)
	at com.microsoft.bingads.internal.ServiceFactoryImpl.createProxyFromService(ServiceFactoryImpl.java:179)

The environment is

Debian
openjdk version "17.0.2" 2022-01-18
OpenJDK Runtime Environment (build 17.0.2+8-86)
OpenJDK 64-Bit Server VM (build 17.0.2+8-86, mixed mode, sharing)

@schabe77
Copy link
Author

I think it's a multi-threading issue. I don't know if this class is meant to be accessed by multiple threads simultaneously.

I think what is happing here

        if(xmlNsCache==null) {
            xmlNsCache = new HashMap<>();
            ...
            Map<String, String> m = xmlNsCache.computeIfAbsent(uri, k -> new HashMap<>());

is that

  • xmlNsCache is null
  • thread 1 is checking -> is null -> steps into if-condition
  • thread 2 is checking -> is null -> steps into if-condition
  • thread 1 is creating HashMap<>() and assigning it to xmlNsCache
  • thread 2 is creating HashMap<>() and assigning it to xmlNsCache (overwriting already assigned xmlNsCache)
  • now thread 1 and thread 2 using the same xmlNsCache when accessing xmlNsCache.computeIfAbsent -> ConcurrentModificationException

Options could be to

  • create ConcurrentHashMap in TypeInfoSetImpl#getXmlNs instead of HashMap
  • initialize xmlNsCache in TypeInfoSetImpl's constructor with ConcurrentHashMap
  • prevent multithreaded use of TypeInfoSetImpl

@jimma
Copy link
Contributor

jimma commented Jun 26, 2024

We met this issue too in CXF. It looks like creating a ConcurrentHashMap for xmlNsCache is good to go

jimma added a commit to jimma/jaxb-ri that referenced this issue Jul 9, 2024
@tomjenkinson
Copy link

tomjenkinson commented Sep 13, 2024

I might have misunderstood what @schabe77 is considering as an option here but if getXmlNs can be called concurrently then I don't think that just making xmlNsCache a ConcurrentHashMap would be enough because multiple threads executing in that method at the same time could see xmlNsCache==null at the same time and thus multiple xmlNsCache = new ConcurrentHashMap<>(); could execute in parallel at the same time, with the latest thread overwriting the variable xmlNsCache /cc @jimma regarding pull request #1806.

@schabe77
Copy link
Author

@tomjenkinson You are right. It is possible that the xmlNsCache is initialized twice. But that's not a (big) problem because it's just a cache. The worst thing that could happen is that a formally cached value would have been re-evaluated. But this fix prevents a RuntimeException, which is much worse.

You would probably have to refactor the class to make it really pretty, but I was more interested in the fix.

@tomjenkinson
Copy link

I think I am with you, sorry for the noise - not synchronizing access allows the xmlNsCache to be overwritten by a competing thread but IIUC from a JAX-B point of view the result should just be considered that the old xmlNsCache is overwritten and left to the garbage collector to deal with. This observation assumes that for this API, it is well defined that HashMaps returned from the call (that are created at

Map<String, String> m = xmlNsCache.computeIfAbsent(uri, k -> new HashMap<>());
) are not expected to be able to be compared with ==/!=.

@tomjenkinson
Copy link

Please can I ask for an update on this and whether there is more information that is needed to be reported to help identify an appropriate solution?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants