I'm sure everyone has seen this thread on fop-users. Any particular preferences on one of the two options I listed below? The first will require coordination with Batik as they are supposed to migrate to that class and it's basically their version. We used a modified version of theirs. The second is probably cleaner but since the change is backwards-incompatible for any ElementMapping implementation (Barcode4J, for example), this is not to be taken lightly. I think the first is easier, should not have any side-effects but feels more like patchwork.
On 15.05.2006 09:27:29 Jeremias Maerki wrote: > > 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