On 13.05.2006 00:45:51 Ryan Gustafson wrote: > Hi Jeremias, > > > The Service class is properly synchronized. > > I downloaded the source, and agree. > > > The ElementMapping > > classes, however, are recreated for each FopFactory. There's no > > sharing of these classes between multiple FopFactory instances. > > I downloaded the XML Graphics Common code. And I've reviewed the > FOP code. I do not see it this way, see my further comments > below... > > > Since you're using the deprecated Fop constructor a new > > FopFactory is instantiated for each new Fop instance. So I don't > > see why the ElementMapping classes need to be thread-safe. > > It's true, we have unique FOP instances, each with unique FopFactory > instances, each with a unique ElementMappingRegistry instance. > Where they all come together is in the ElementMappingRegistry static > call to Service.providers(), which based on the code I saw in the > XML Graphics Common 1.0 version of Service.java, would appear to > return an Iterator over the _same_ statically cached list List, > which then contains the same ElementMapping objects. Thus, the > FopFactory objects end up using the same ElementMapping objects. > > The Service code appears to be using standard ArrayList and HashMap, > so the Map.put()/Map.get() and List.iterator() method work normally > (no behind the scenes cloning/copying going on). Further, it is > directly placing a normal FOElementMapping instance into the list > (no special wrapping going on).
Right, I should have placed a breakpoint earlier to see what happens. I was under the assumption that only the class names are stored in the list. The reason for that: We used a modified version of Batik's Service class which only saved the class names. When I populated XML Graphics Commons I took Batik's implementation and adjusted FOP's code to use that. Turns out this wasn't without side-effects. From this POV, the multi-threading issue is absolutely logical. > Should the Service be returning caller unique instances in the > Iterator? If so, it needs to be re-written. The JavaDoc doesn't > give me the feeling that is supposed to do this however. I see two possible routes: 1. Adding an optional parameter to the Service class' providers() method which allows to return class names instead of instances. This will restore the previous behaviour and maintain backwards compatibility with the existing ElementMapping implementations. 2. Change ElementMapping's constructor to accept the namespace URI as a parameter and call initialize() right after that. Drawback: It's a backwards-incompatible change. We'll discuss that on fop-dev and decide what to do. > I only spent about 20 minutes looking over this, so I may have > overlooked something? > > > I don't buy it, yet. I can see from your stack trace that > something's > > wrong but I'm not sure you've found the right spot. What you can > > try is to synchronize ElementMapping.getTable() and check if this > > changes anything, but I just don't understand why two threads > > would modify the same HashMap in ElementMapping. > > > > I wish I could reproduce the problem. I've tried yesterday, but > > didn't succeed. I ran up to 60 threads in parallel with the > > deprecated constructor and had no problems. But on the other > > hand, I only had a single-processor machine to test on. Are you > > on a multi-processor box > > Yup, multi-processor box. That definately makes it easier to expose > thread safety issues. I believe we were running about 5 thread max > through the FOP code. Also we've only seen it once so far, it may > be tricky to trigger. > > Tell you what, I'm due to take over working on our FOP related code > in a couple weeks. When I do, I'll keep my eye out for this issue, > and if I find out anything more, I'll do what I can to find the > source and share what I find. > > In the meantime, we'll move to the non-deprecated API, and presume > the issue is solved, until we get more Exceptions to prove > otherwise. Certainly a good move. I believe I know now what's wrong so we can take appropriate action. Thanks for being so persistent. Jeremias Maerki --------------------------------------------------------------------- To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
