conversation starter about behaviour changes introduced in fontbox 3.x

[pcorless]

I work on a library that leverages fontbox for reading fonts. The 3.x release introduced a change that breaks my rendering tests. I finally had a chance to dig a little deeper as to what might have changed between 2.x and 3.x as I'd like to migrate to 3.x. I believe PDFBOX-5143 (reuse Type2CharStringParser instead of recreating it for every char) may have introduced the issue I'm seeing.

The issue is relate to how I call fontbox api from multiple threads, or to say, make render calls to the same font from multiple threads. The PR shows a very simplistic work around to the problem that gets things rendering correct in my test suite.

Could you let me know if you'd entertain some type of synchronization to make the calls more or less thread safe? If so, I can build out a more robust solution with supporting tests.

[THausherr]

I don't mind adding synchronized. But wouldn't this invalidate your multithread strategy, which (I guess) is to get the path of each glyph in parallel?

[pcorless]

Ideally it be parallel. When the issue manifests itself the glyphs are corrupted or the there is a concurrent modification exception thrown. I'll take another look at the parser class and see if there might be another way of going about the issue.

[lehmi]

IMHO to synchronize the part of the code in question may have some downsides. It can slow down the processing and in a worst case scenario it might run into a deadlock.
I'd prefer to make the parser thread-safe by eliminating the private members fields of the class. I'm going to open a JIRA ticket

[THausherr]

@pcorless can you create a simple parallel test that fails with the current code? There is an otf font at resources/otf/FoglihtenNo07.otf which is CFF based.

[lehmi]

Done, I've removed the private member fields, so that the class should be thread-safe again, see https://issues.apache.org/jira/browse/PDFBOX-5819

[pcorless]

Thank you for addressing this issue so quickly.

I've updated the PR with a little unit test that can reproduce the the issue.

[THausherr]

Nice. I can confirm that is fails with 3.0.2, succeeds with 2.0.31, and succeeds with the latest change in the trunk.

[lehmi]

@pcorless thanks for the feedback and the test.