Castor is not thread safe.

[Dinesh-Khandelwal]

We are using castor-xml-1.3.2.jar for Xml data binding. In production we had an issue where the amount field which is of type BigDecimal, was getting swapped with other amount value from different xml document and thread at the time of un-marshalling. When we analyzed further we found the issue with PrimitiveObjectFactory class which internally has static Map of handlers to handle different data types. This handler is used to create the respective data type objects with the values passed from SAX parser. But the handler maintains a state with fields Class<?> type; String value; Object object; in PrimitiveObject class which is super class of all the handlers and which is not thread safe.

Earlier we thought that the issue is only for Big-Decimal but its is for any other data type as well.

[wguttmn]

Can you please provide a pull request that (at least) clearly identifies the code at question ? And a fully fledged test case to replay the problem would be of great help as well.

[Dinesh-Khandelwal]

I have sent u a mail with test-cases attached. I am not able to attach the zip file with test-cases here.

[Dinesh-Khandelwal]

I see the code of castor-xml-1.4.0-sources.jar and its fixed. I would like to know whether its officially supported version and what all fixes are done from 1.3.2 to 1.4. Do we have any later version to 1.4?

[wguttmn]

Please have a look at http://castor-data-binding.github.io/castor/main/index.html for version information

[wguttmn]

Due to our (forced) move from Codehaus to github.com, some of that release information is not easily available.

[Dinesh-Khandelwal]

Thanks a lot.

[Dinesh-Khandelwal]

would like to know if Castor is still supported actively?

[wguttmn]

So releases 1.4 and 1.4.1 are not indication enough ?

[wguttmn]

Incl. the move to github, etc ?

[Dinesh-Khandelwal]

yes , make sense. It was just an interest to know as many people think the support is not there.

[Dinesh-Khandelwal]

We are using Castor 1.3.2 and faced so many performance issues with that. We have made 4-5 changes and it really scaled well. I would like to share with you one of them..

As Castor team suggests to use single XMLContext object so that we could make use caching of descriptors but that is not totally true. As when the public void addMapping(final Mapping mapping) throws MappingException is used Castor creates a new XMLContext object which defeats the purpose of caching for subsequent request.

The old Castor code :
public void addMapping(final Mapping mapping) throws MappingException { MappingUnmarshaller mappingUnmarshaller = new MappingUnmarshaller(); MappingLoader mappingLoader = mappingUnmarshaller.getMappingLoader(mapping, BindingType.XML); _internalContext.getXMLClassDescriptorResolver() .setMappingLoader(mappingLoader);
}

The new code now we have :
public void addMapping(final Mapping mapping) throws MappingException { MappingUnmarshaller mappingUnmarshaller = new MappingUnmarshaller(_internalContext); MappingLoader mappingLoader = mappingUnmarshaller.getMappingLoader(mapping, BindingType.XML); _internalContext.getXMLClassDescriptorResolver().setMappingLoader(mappingLoader); }

[wguttmn]

It was just an interest to know as many people think the support is not there. Who's many ? I mean, I heave been committer for 10+ years, and admittedly I have seen other committers coming and going :-(.

But with the move to github, it's infrastructure, it's so easy to contribute, and there's some people who have done so already.

[wguttmn]

We have made 4-5 changes and it really scaled well. I would like to share with you one of them.. Why not share all of them in form of a patch/pull request ?

[wguttmn]

As Castor team suggests to use single XMLContext object so that we could make use caching of descriptors but that is not totally true. As when the

Can you please create a separate issue for this and attach a pull request. I will review, comment, have some questions .... but in the end merge things into the code base.

[wguttmn]

Given that the original problem at hand has been resolved in 1.4.x already, can I assume it's safe to close this issue ?

[Dinesh-Khandelwal]

yes , you could close the earlier issue which I raised in the starting "swapping of values". And about sharing those fixes which we did , I will definitely share with you all of them.

[Dinesh-Khandelwal]

could u please guide me on how to create a pull request and commit those changes for your reference. I am new to GitHub.

[wguttmn]

Could you please have a general look at https://help.github.com/articles/proposing-changes-to-a-project-with-pull-requests, and we take it from there ?

[wguttmn]

Or even better here: https://help.github.com/categories/collaborating-on-projects-using-issues-and-pull-requests/

[wguttmn]

Basically, fork the Castor repo so that you have a fork of the Castor repo for which you have all right.

When working on something, create a branch in your repo, make the changes on this branch, test them.

Then create a pull request in Castor against your branch.

Please always keep the changes to a bare minimum, so that the pull requests do not get too big and diluted.

Doe this make sense ?

[wguttmn]

Basically, fork the Castor repo so that you have a fork of the Castor repo for which you have all right.

When working on something, create a branch in your repo, make the changes on this branch, test them.

Then create a pull request in Castor against your branch.

Please always keep the changes to a bare minimum, so that the pull requests do not get too big and diluted.

Doe this make sense ?

[wguttmn]

Closing this as discussed.

[Dinesh-Khandelwal]

create the pull request with all the performance fixes. https://github.com/Dinesh-Khandelwal/castor/pull/1

[Dinesh-Khandelwal]

https://github.com/castor-data-binding/castor/pull/61