Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FontUtils.setSansFontsAsDefault is not thread-safe #272

Open
stanislaw89 opened this issue Oct 30, 2023 · 1 comment
Open

FontUtils.setSansFontsAsDefault is not thread-safe #272

stanislaw89 opened this issue Oct 30, 2023 · 1 comment
Labels

Comments

@stanislaw89
Copy link

The FontUtils.setSansFontsAsDefault is implemented as

    private static final Map<String, PDFont> defaultFonts = new HashMap<>();
    ...
    public static void setSansFontsAsDefault(PDDocument document) {
		defaultFonts.put("font", loadFont(document, "fonts/FreeSans.ttf"));
		defaultFonts.put("fontBold", loadFont(document, "fonts/FreeSansBold.ttf"));
		defaultFonts.put("fontItalic", loadFont(document, "fonts/FreeSansOblique.ttf"));
		defaultFonts.put("fontBoldItalic", loadFont(document, "fonts/FreeSansBoldOblique.ttf"));
    }

Then it's used like

	public Paragraph(...) {
	        ...
		// check if we have different default font for italic and bold text
		if (FontUtils.getDefaultfonts().isEmpty()) {
			fontBold = PDType1Font.HELVETICA_BOLD;
			fontItalic = PDType1Font.HELVETICA_OBLIQUE;
			fontBoldItalic = PDType1Font.HELVETICA_BOLD_OBLIQUE;
		} else {
			fontBold = FontUtils.getDefaultfonts().get("fontBold");
			fontBoldItalic = FontUtils.getDefaultfonts().get("fontBoldItalic");
			fontItalic = FontUtils.getDefaultfonts().get("fontItalic");
		}
		...
	}

There are two issues related to multi-thread environments.

First, the defaultFonts map is not thread-safe; it's possible to get a ConcurrentModificationException if FontUtils.setSansFontsAsDefault is called from different threads simultaneously.

The second issue is that fonts are shared between documents.
When we call FontUtils.setSansFontsAsDefault(document), it may look like we configure the default fonts for the document passed as the parameter. However, this is not true; the document used to load the font (PDType0Font), which keeps the reference to the document (PDType0Font.embedder->document). As a result, the mutable non-thread safe set TrueTypeEmbedder.subsetCodePoints is accessed via multiple threads and, therefore, throws ConcurrentModificationException.

@johnmanko
Copy link
Collaborator

@stanislaw89 Can you provide a PR for this? Changing the Map to a ConcurrentMap is the easy part. Is there a way to disconnect the loading document from the returned PDType0Font? Using the returned value from .load to popular the map might have to be discarded in favor of direct loading the file. I don't know, but I'd like to see a solution to this.

@johnmanko johnmanko added the bug label Nov 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants