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
