Re: [Cdk-devel] Thread safety issues in org.openscience.cdk.smsd

[John M. <joh...@gm...>]

Hi Scooter,

Thanks for the patch, could you submit it to http://sourceforge.net/p/cdk/patches where I can review and sign it off? If I'm the one who adds it and signs it off it'll get pulled slower :-).

As you say the code is legacy, in fact, that SMSD code 2+ years out of date and you'll probably find most of the issues have been fixed on the actual SMSD project: https://github.com/asad/smsd. You can exclude the cdk-smsd module and use the SMSD project library instead. That said, I believe the CDK version used in the SMSD code is /master (i.e. 1.5.x, unstable). You can use master in production code but you'll have to patch out some select changes. The main issues are with query molecules but these pop up more then you'd expect.

Asad can give more info on whether SMSD is currently using master/1.4.x….

J

On 14 Feb 2013, at 17:24, Scooter Morris <sc...@cg...> wrote:

> Hi all,
>    I've been adding the capability to calculate the most common substructure to the chemViz plugin in Cytoscape and I've run into some thread-safety issues in org.openscience.cdk.smsd.  In particular, there are a number of static routines and variables that don't need to be static and cause real issues when run in multiple threads.  For example, in org.openscience.cdk.smsd.algorithm.rgraph.CDKRMapHandler, there are a number of static routines that are used to return the source and target of the calculation.  In fact, these don't need to be static -- nothing in that class is ever called as a static and because those routines are static, I was getting the wrong IAtomContainers returned -- leading to exceptions in a multi-threaded environment.
>    This is easily fixed by removing all of the statics, but I noticed that are other instances of static methods scattered throughout the smsd package.  I'm sure this is legacy given the history of smsd merging into cdk, but it would be really nice to clean up.  I have a set of diffs for CDKRMapHandler.java that fix my specific problem (attached), but I suspect that there are more lingering.  I'm happy to take this on, if no one has any objection...
> 
> -- scooter
> <threadDiffs.out>------------------------------------------------------------------------------
> Free Next-Gen Firewall Hardware Offer
> Buy your Sophos next-gen firewall before the end March 2013 
> and get the hardware for free! Learn more.
> http://p.sf.net/sfu/sophos-d2d-feb_______________________________________________
> Cdk-devel mailing list
> Cdk...@li...
> https://lists.sourceforge.net/lists/listinfo/cdk-devel