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

#21 java.lang.NullPointerException: Cannot invoke "java.util.Map.get(Object)" because "this.map" is null #80

Closed
wants to merge 1 commit into from

Conversation

arnosthavelka
Copy link

@arnosthavelka arnosthavelka commented Nov 10, 2021

Some time ago, I have found this error on Java 15 (Java 14 seemed OK) when I upgraded to this version. Unfortunately, it's not enough, because the incorrect initialization just moved to other part. See my arnosthavelka/itext-poc#21 with more details.

The Java specification declares: A blank final instance variable must be definitely assigned (§16.9) at the end of every constructor (§8.8) of the class in which it is declared; otherwise a compile-time error occurs.. Clearly the new JDK tries to optimize initialization and doesn't wait before passing the instance before it's really initialized. Therefore, we have unstable code. Sometimes it's OK and sometimes it just fails.

See: https://docs.oracle.com/javase/specs/jls/se17/html/jls-8.html#jls-8.3.1.2 or https://stackoverflow.com/questions/11345061/why-must-a-final-variable-be-initialized-before-constructor-completes

The full stacktrace looks like:

java.lang.NullPointerException: Cannot invoke "java.util.Map.get(Object)" because "this.map" is null
	at com.itextpdf.kernel.pdf.PdfDictionary.get(PdfDictionary.java:463)
	at com.itextpdf.kernel.pdf.PdfDictionary.getAsArray(PdfDictionary.java:161)
	at com.itextpdf.kernel.pdf.PdfPage.getMediaBox(PdfPage.java:569)
	at com.itextpdf.kernel.pdf.PdfPage.getPageSize(PdfPage.java:135)
	at com.itextpdf.kernel.pdf.PdfPage.getPageSizeWithRotation(PdfPage.java:144)
	at com.github.aha.poc.itext.DocumentBuilder.addWatermarkToPage(DocumentBuilder.java:249)
	at com.github.aha.poc.itext.DocumentBuilder.addWatermark(DocumentBuilder.java:163)
	at com.github.aha.poc.itext.WatermarkTests.watermark(WatermarkTests.java:26)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:77)
	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.base/java.lang.reflect.Method.invoke(Method.java:568)
	at org.junit.platform.commons.util.ReflectionUtils.invokeMethod(ReflectionUtils.java:688)
	at org.junit.jupiter.engine.execution.MethodInvocation.proceed(MethodInvocation.java:60)
	at org.junit.jupiter.engine.execution.InvocationInterceptorChain$ValidatingInvocation.proceed(InvocationInterceptorChain.java:131)
	at org.junit.jupiter.engine.extension.TimeoutExtension.intercept(TimeoutExtension.java:149)
	at org.junit.jupiter.engine.extension.TimeoutExtension.interceptTestableMethod(TimeoutExtension.java:140)
	at org.junit.jupiter.engine.extension.TimeoutExtension.interceptTestMethod(TimeoutExtension.java:84)
	at org.junit.jupiter.engine.execution.ExecutableInvoker$ReflectiveInterceptorCall.lambda$ofVoidMethod$0(ExecutableInvoker.java:115)
	at org.junit.jupiter.engine.execution.ExecutableInvoker.lambda$invoke$0(ExecutableInvoker.java:105)
	at org.junit.jupiter.engine.execution.InvocationInterceptorChain$InterceptedInvocation.proceed(InvocationInterceptorChain.java:106)
	at org.junit.jupiter.engine.execution.InvocationInterceptorChain.proceed(InvocationInterceptorChain.java:64)
	at org.junit.jupiter.engine.execution.InvocationInterceptorChain.chainAndInvoke(InvocationInterceptorChain.java:45)
	at org.junit.jupiter.engine.execution.InvocationInterceptorChain.invoke(InvocationInterceptorChain.java:37)
	at org.junit.jupiter.engine.execution.ExecutableInvoker.invoke(ExecutableInvoker.java:104)
	at org.junit.jupiter.engine.execution.ExecutableInvoker.invoke(ExecutableInvoker.java:98)
	at org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.lambda$invokeTestMethod$6(TestMethodTestDescriptor.java:210)
	at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
	at org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.invokeTestMethod(TestMethodTestDescriptor.java:206)
	at org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.execute(TestMethodTestDescriptor.java:131)
	at org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.execute(TestMethodTestDescriptor.java:65)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$5(NodeTestTask.java:139)
	at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$7(NodeTestTask.java:129)
	at org.junit.platform.engine.support.hierarchical.Node.around(Node.java:137)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$8(NodeTestTask.java:127)
	at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.executeRecursively(NodeTestTask.java:126)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.execute(NodeTestTask.java:84)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)
	at org.junit.platform.engine.support.hierarchical.SameThreadHierarchicalTestExecutorService.invokeAll(SameThreadHierarchicalTestExecutorService.java:38)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$5(NodeTestTask.java:143)
	at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$7(NodeTestTask.java:129)
	at org.junit.platform.engine.support.hierarchical.Node.around(Node.java:137)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$8(NodeTestTask.java:127)
	at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.executeRecursively(NodeTestTask.java:126)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.execute(NodeTestTask.java:84)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)
	at org.junit.platform.engine.support.hierarchical.SameThreadHierarchicalTestExecutorService.invokeAll(SameThreadHierarchicalTestExecutorService.java:38)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$5(NodeTestTask.java:143)
	at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$7(NodeTestTask.java:129)
	at org.junit.platform.engine.support.hierarchical.Node.around(Node.java:137)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$8(NodeTestTask.java:127)
	at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.executeRecursively(NodeTestTask.java:126)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.execute(NodeTestTask.java:84)
	at org.junit.platform.engine.support.hierarchical.SameThreadHierarchicalTestExecutorService.submit(SameThreadHierarchicalTestExecutorService.java:32)
	at org.junit.platform.engine.support.hierarchical.HierarchicalTestExecutor.execute(HierarchicalTestExecutor.java:57)
	at org.junit.platform.engine.support.hierarchical.HierarchicalTestEngine.execute(HierarchicalTestEngine.java:51)
	at org.junit.platform.launcher.core.EngineExecutionOrchestrator.execute(EngineExecutionOrchestrator.java:108)
	at org.junit.platform.launcher.core.EngineExecutionOrchestrator.execute(EngineExecutionOrchestrator.java:88)
	at org.junit.platform.launcher.core.EngineExecutionOrchestrator.lambda$execute$0(EngineExecutionOrchestrator.java:54)
	at org.junit.platform.launcher.core.EngineExecutionOrchestrator.withInterceptedStreams(EngineExecutionOrchestrator.java:67)
	at org.junit.platform.launcher.core.EngineExecutionOrchestrator.execute(EngineExecutionOrchestrator.java:52)
	at org.junit.platform.launcher.core.DefaultLauncher.execute(DefaultLauncher.java:96)
	at org.junit.platform.launcher.core.DefaultLauncher.execute(DefaultLauncher.java:84)
	at org.eclipse.jdt.internal.junit5.runner.JUnit5TestReference.run(JUnit5TestReference.java:98)
	at org.eclipse.jdt.internal.junit.runner.TestExecution.run(TestExecution.java:40)
	at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:529)
	at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:756)
	at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.run(RemoteTestRunner.java:452)
	at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.main(RemoteTestRunner.java:210)

@arnosthavelka
Copy link
Author

arnosthavelka commented Nov 10, 2021

Unfortunately, the last issue is more complicated as I'm not familiar with your code structure yet. I wasn't able to identify the wrong place so far.

com.itextpdf.kernel.exceptions.PdfException: Invalid PDF. There is no media box attribute for page or its parents.
	at com.itextpdf.kernel.pdf.PdfPage.getMediaBox(PdfPage.java:574)
	at com.itextpdf.kernel.pdf.PdfPage.getPageSize(PdfPage.java:135)
	at com.itextpdf.kernel.pdf.PdfPage.getPageSizeWithRotation(PdfPage.java:144)
	at com.github.aha.poc.itext.DocumentBuilder.addWatermarkToPage(DocumentBuilder.java:249)

@Snipx
Copy link
Contributor

Snipx commented Nov 10, 2021

Hi @arnosthavelka and thanks for you PR! Would you mind signing the Contributor's License Agreement? https://itextpdf.com/en/how-buy/legal/itext-contributor-license-agreement

@Snipx
Copy link
Contributor

Snipx commented Nov 10, 2021

On the PR itself: Could you add a single-threaded test demonstrating the issue? PdfDictionary is not supposed to be thread-safe at the moment, and I think that field initialization happens before any method is called (see https://docs.oracle.com/javase/specs/jls/se8/html/jls-12.html#jls-12.4.2):

...
Next, execute either the class variable initializers and static initializers of the class, or the field initializers of the interface, in textual order, as though they were a single block.
...

The problem you are facing could be a side effect of a testing framework that uses multithreaded environment, in that case we will have to consider this case more carefully.

The SO question you refer to is about compilation errors which we don't have. The quote from Oracle spec is also about a potential compilation error if you don't assign a final field before constructor ends, so could you elaborate on the connection of the links you mention to the issue you are experiencing?

@arnosthavelka
Copy link
Author

Hi @arnosthavelka and thanks for you PR! Would you mind signing the Contributor's License Agreement? https://itextpdf.com/en/how-buy/legal/itext-contributor-license-agreement

sure, I'll do it soon

@arnosthavelka
Copy link
Author

Could you add a single-threaded test demonstrating the issue?

To prove / support my issue I changed my test from @Test to @RepeatedTest(10). See the random failures.
image

It's true, it can be still caused by my code -> even so it seems unlikely to me ;-).

I'll try to provide simple test as requested. However, I cannot guarantee when.

PdfDictionary is not supposed to be thread-safe
It's not my case. My test is single threaded instantiating PdfDocument as:

	PdfDocument createDocument(String targetFilename, WriterProperties writerProperties) {
		try {
			PdfWriter writer = new PdfWriter(targetFilename, writerProperties);
			return new PdfDocument(writer);
		} catch (FileNotFoundException e) {
			log.error("Creating PDF failed", e);
			throw new ITextException(e.getMessage());
		}
	}

The problem you are facing could be a side effect of a testing framework

I don't think so. But let's see with my promised test example.

I think that field initialization happens before any method ...

Well, I have 2 objections here:

  1. You are referring to JDK8 specification, but I have pointed out that it was working with this version. It started from JDK15 in my case. See https://docs.oracle.com/javase/specs/jls/se17/html/jls-12.html#jls-12.4.2.
  2. This part talks about class initialization. There's no clue when is the instance passed to the client's code (who instantiated) to be able to invoke any method on it.

... Oracle spec is also about a potential compilation error...

I'm not sure I understand your point. However, see the change on line 499 in my PR. Such change was forced by the compiler -> to prevent having a null on a final field.

@arnosthavelka
Copy link
Author

Here is the requested test:

class ITextTests {

	@RepeatedTest(10)
	void watermark() throws Exception {
		var text1 = """
				Duisnetus ceteros proin posuere at invidunt purus minim adolescens vehicula.  Sempervero graeci definiebas definitiones errem egestas utamur assueverit delectus quem esse meliore neglegentur porta neque signiferumque ius elitr.  Dictassaperet scripta perpetua habeo risus tation definitiones fames expetendis verterem alienum duo erroribus detracto.
				Fabellasalia has simul litora ornatus recteque senserit morbi evertitur inani voluptaria fuisset referrentur qui.  Voluptariadisputationi ridens indoctum delectus fabellas urna eripuit vulputate porttitor evertitur alterum ignota inciderint justo latine dico duo melius aptent.  Dicoponderum quaerendum fusce tibique habitasse posse quem quod nisi elitr blandit placerat suavitate.  Atnunc voluptatum nihil quisque nominavi dolorum intellegat natum eius molestiae iusto sententiae melius eos quaerendum primis decore maximus praesent.  Aequeac non curabitur sodales dapibus placerat dicam ferri.
				Vestibulumoption evertitur menandri inimicus mauris nonumy appareat conceptam definitiones nullam invidunt dicta ocurreret nascetur porta sapien.  Facilisidelicata luctus conclusionemque sodales pharetra iuvaret option evertitur error velit invidunt tristique evertitur an pertinax integer adolescens facilis tota.  Repudiandaepulvinar ad.  Meldictas suscipiantur definitionem dis moderatius porttitor adipiscing recteque scripta.  Causaealiquam periculis pertinacia facilisi elaboraret postulant venenatis veri docendi agam pertinacia habemus mollis lobortis harum.  Sapientemarcu vehicula consectetuer ocurreret legimus elementum agam.
				Duiselectram sollicitudin rhoncus montes contentiones postulant sagittis omittam alterum equidem.  Adversariumligula iisque verterem efficitur patrioque molestiae gloriatur elementum vocent.
				Justovivendo senectus dictas taciti posuere mel maximus regione leo signiferumque.  Meliusturpis aliquip perpetua inceptos commune legimus utamur natum inimicus recteque justo dictumst.  Offenditfusce blandit erroribus accumsan reprehendunt pericula cras nullam lobortis necessitatibus pulvinar pericula possit recteque velit mattis.  Doloremposse praesent sanctus graece meliore viverra evertitur maiestatis pro tristique tellus convallis solum reformidans quot tacimates magnis mediocrem contentiones.  Voluptatuminteger veri.  Tempuspostea dolorum elaboraret reque vocent voluptaria donec conubia.
				Inconvenire pro fuisset qui utroque nobis repudiandae quam dicat mediocritatem unum elaboraret velit.  Omittanturei homero expetenda utinam vestibulum oporteat orci verterem legimus suas causae vehicula vituperata omittantur dicta.  Fabellaspellentesque dolorum ignota erat eros suas eirmod urna similique consequat orci taciti neglegentur melius eget orci natoque.
				Docendiesse dapibus repudiare porta molestie ignota.  Omnesqueclass veritus scelerisque alienum aperiri sententiae amet omittantur urna.  Idquepopulo iriure quaerendum inani duo hac ocurreret duis mucius scelerisque ac oporteat mediocrem quaestio tristique appareat fabulas etiam.  Vixpellentesque simul saperet facilis eius accusata mediocrem ne quidam repudiare vocibus curabitur consequat pericula.
				Ridensequidem pharetra tristique dicam.  Intellegebatnostrum iisque habemus platonem iriure sociosqu novum eloquentiam nonumy mentitum instructior postea appetere vidisse constituto dolor scelerisque delicata conclusionemque.  Ornatusomnesque tota magna habeo mediocritatem felis tempor splendide molestiae hinc.  Fuissetmediocritatem torquent partiendo delectus mel tempor quo semper pellentesque maluisset vituperatoribus lacus class mus.  Appeteresonet eum decore numquam atqui luptatum vis his expetenda viverra aliquet necessitatibus fuisset malesuada mandamus parturient rhoncus atqui.  Euismodquod facilisis scripserit pulvinar maecenas esse facilis alienum ut netus tortor moderatius scripserit.
				Interessetfacilisis dico dictum elaboraret.  Librissagittis vero ubique adversarium quidam pretium molestie sagittis mazim contentiones venenatis gravida sem alienum regione nihil porttitor mauris.  Constituamneque constituam.
				""";
		var text2 = """
				Mattisne dui imperdiet mea suscipiantur pri suas evertitur duo suscipit.  Proinfeugait est.  Quasdolore lacinia tale pellentesque legere mea atomorum sumo no felis dicam intellegebat curabitur maiestatis sumo malesuada appetere mea.  Oporteatpropriae facilis iriure nunc neque omnesque purus persecuti eleifend interesset iuvaret imperdiet.  Euripidiseuismod pertinax ponderum dictumst tempus morbi.
				Epicurivocibus propriae detraxit sed sonet vis mei congue dolorem quam eirmod platea saepe appetere ceteros.  Crasubique ubique.  Blanditcurabitur nostra aptent mutat parturient interpretaris ut dicat posse ultrices gloriatur no ceteros montes causae.  Veritusmel possit facilis possit tristique epicurei nonumy volumus veritus id per legimus petentium qui definitionem.
				Voluptatibusbibendum postea porta taciti dictas convenire malesuada atqui dui ocurreret noluisse natoque constituto tortor orci netus.  Ignotautinam maluisset in vis morbi est reprehendunt offendit posidonium aliquip accommodare agam sadipscing graeco.
				Bibendumadipisci felis pellentesque pertinacia honestatis erat quem erat porttitor mollis iusto sonet error et.  Intellegebatpraesent reprehendunt constituto splendide posidonium vero dissentiunt repudiandae sagittis.  Pharetravelit euripidis.  Doloreet sociosqu unum.  Senectusper iudicabit oratio congue persequeris interpretaris consectetur causae delenit convenire aliquet libris mel maluisset nonumes possim.
				Neea senserit audire iusto aperiri ut pertinacia iudicabit vocibus moderatius proin.  Requetheophrastus recteque postulant an repudiandae constituto disputationi elitr invidunt quo omittam postulant.  Arcuperpetua graecis proin sit elitr explicari assueverit mattis vis sanctus ea.
				Hasvolumus gubergren consul.  Pertinaciautinam vivendo pulvinar odio sagittis reformidans offendit civibus ultricies maiorum voluptaria tacimates himenaeos ei tortor solet odio eos enim.  Elementumdelectus a nostrum magna eu autem eius aptent reformidans gravida propriae his movet deterruisset.  Maiorumproin volumus porttitor wisi mus.
				Posteanam commune consequat verear vocibus sit vulputate proin mutat ferri finibus dicunt tristique an diam errem his delectus.  Adversariumporro volutpat voluptatibus vocibus nobis cum qui integer mi.  Utauctor penatibus est simul magnis pericula delectus quidam dignissim sodales dolorum invenire ac tellus sociosqu viris repudiare iuvaret.  Referrenturante parturient omnesque graece habitant tota elementum nullam gravida mel utamur usu.  Egetdolore felis quisque laoreet magna explicari noluisse orci habemus sanctus ne error brute suscipiantur vestibulum ut evertitur.
				""";
		var text3 = """
				Permolestiae electram doctus lacinia solet.  Ultricieseuripidis class animal deterruisset pharetra accusata qualisque faucibus eum.  Tacitiutroque invenire lacinia elit fastidii falli dolor vocent ancillae cursus accumsan dolorum eirmod civibus deseruisse.  Mediocritatemlacinia potenti ea euismod iudicabit novum sodales feugiat accumsan posuere ridens curabitur leo sociosqu.  Invenirecontentiones disputationi habitant viverra malorum definitionem reque delicata.
				Minimpetentium convallis vituperatoribus tempus deseruisse decore mea novum gubergren congue percipit dictumst verterem mediocritatem quas dapibus definitiones dicam menandri.  Consequatcurae audire mutat te.  Posteaeu sapien tacimates vehicula turpis sed brute convenire civibus populo dicta repudiandae appetere honestatis.  Quiduo conceptam curabitur mus voluptatum idque his adipiscing habemus harum suavitate nostrum saepe ex egestas minim mei taciti.  Gravidavel invidunt varius accumsan definitionem no electram.  Essedicit consectetur voluptatibus viris pri consectetur voluptaria utinam noster.
				Veriaugue molestiae ubique mediocritatem ponderum discere.  Facilisiegestas voluptatibus volutpat.  Voluptatibusfringilla saperet platonem per curabitur accommodare ubique et nam magnis curabitur.  Neglegenturfacilisis risus expetendis mazim eum commodo perpetua scripserit porta contentiones scripta idque.  Verodolores noluisse offendit persequeris adipisci graece cras gravida voluptatibus.
				Vituperatoribusreprimique a pro vestibulum ut maiorum patrioque verear feugiat propriae nihil posse euripidis vulputate.  Laciniasit principes equidem vis tibique ubique sodales ipsum cras netus.  Minimpropriae tacimates quas ludus doming has vero adversarium fames ius.  Quodsodales per felis.  Tibiqueintellegebat dapibus detracto vituperatoribus pretium audire donec diam persius sed vidisse commune ipsum quaestio error.  Dislabores varius unum orci persecuti sagittis definitionem enim commodo corrumpit.
				""";

		var pdfWriter = new PdfWriter("target/example-watermark.pdf");
		var document = new Document(new PdfDocument(pdfWriter));
		document.add(new Paragraph(text1));
		document.add(new Paragraph(text2));
		document.add(new Paragraph(text3));

		Paragraph paragraph = new Paragraph("java.lang.NullPointerException: Cannot invoke \"java.util.Map.get(Object)\" because \"this.map\" is null");

		PdfExtGState transparentGraphicState = new PdfExtGState().setFillOpacity(0.5f);
		for (int i = 1; i <= document.getPdfDocument().getNumberOfPages(); i++) {
			addWatermarkToPage(document, i, paragraph, transparentGraphicState);
		}

		document.close();
	}

	private void addWatermarkToPage(Document document, int pageIndex, Paragraph paragraph, PdfExtGState graphicState) {
		PdfDocument pdfDoc = document.getPdfDocument();
		PdfPage pdfPage = pdfDoc.getPage(pageIndex);
		Rectangle pageSize = pdfPage.getPageSizeWithRotation();

		float x = (pageSize.getLeft() + pageSize.getRight()) / 2;
		float y = (pageSize.getTop() + pageSize.getBottom()) / 2;
		PdfCanvas over = new PdfCanvas(pdfDoc.getPage(pageIndex));
		over.saveState();
		over.setExtGState(graphicState);
		document.showTextAligned(paragraph, x - 50, y, pageIndex, CENTER, TOP, 45);
		over.restoreState();
	}

}

Notes:

  1. When I use it on iText 7.2.0 then I always get the reported error java.lang.NullPointerException: Cannot invoke "java.util.Map.get(Object)" because "this.map" is null
  2. When I use my fixed (version 7.2.1-SNAPSHOT with the changes from this PR) then I'm getting Invalid PDF. There is no media box attribute for page or its parents. -> as mentioned above
  3. When I comment out adding of any paragraph or I just use short text (for the paragraph) then the test succeds, no matter what version is used -> it seems related to the number of pages.

@arnosthavelka
Copy link
Author

well, I've played with the test more with these discoveries:

  1. It's probably not related to initialization (I tired to delay the processing) and neither to JDK version (I guess it was just coincidence that I realized / mentioned the issue after the JDK upgrade).
  2. Adding watermarks works correctly for 2 pages, but it always fails for 3 and more pages.
  3. I tried to follow iText guide https://kb.itextpdf.com/home/it7kb/examples/watermark-examples to add watermark feature. Therefore, I hope the code itself is OK on my side.

I believe the PR is correct -> as the map is not supposed to be null. However, the root cause of this issue is probably hidden somewhere else. The map is probably null due to some other issue.

@Snipx
Copy link
Contributor

Snipx commented Nov 20, 2021

Thanks for sharing the code snippet @arnosthavelka!
I have been able to reproduce the exception you are getting.

The root cause though is not in the fact that the map field is not getting initialized before get() call - I still strongly believe Java guarantees such a field is initialized before its first usage. The root cause is that iText sets map to null in releaseContent() which is expected behavior when we flush page content and its resources to optimize memory consumption.

What could definitely be improved of course is the exception you are getting - it's not super user friendly to just get an NPE. But you are dealing with the same document on multiple levels (higher layout level and lower kernel level) so those things on the edge might shoot out sometimes in case there is a problem in the original code.

That being said, the problem is just because you are iterating over all pages after they have been created, and for documents with >=3 pages iText has already flushed first couple of pages by the time you reached your processing in for loop to optimize memory.

The fix of the code is pretty simple - Document document = new Document(new PdfDocument(pdfWriter)); -> Document document = new Document(new PdfDocument(pdfWriter), PageSize.A4, false); to disable immediate flush that optimizes memory consumption.

@arnosthavelka
Copy link
Author

Thanks for letting me know the real issue.

The root cause though is not in the fact that the map field is not getting initialized before get() call

I know. I realized that while preparing the example code.

for flushing) Isn't it iText core feature I shouldn't care about? It can be an issue when generating a huge PDF with watermark, cannot it?

for this PR ) Will you accept it? Otherwise I can close it. As I already wrote I believe it's an improvement. It prevents NPE as in my case. It doesn't matter what's the cause of NPE, but you logic rely on not null field. Therefore, it should be ensured with the final word.

@Snipx
Copy link
Contributor

Snipx commented Nov 23, 2021

Isn't it iText core feature I shouldn't care about?

It depends on your goal I think. In some cases when you operate on lower levels you should; in some cases (like yours probably) I agree that it creates undesired behavior at times. Exception should be clear, no doubt about that. In terms of the behavior itself, 95% of the users don't operate on multiple levels, and want their layout code to be concise, quick and performant and this is what happens now. Additional post-processing on lower levels like in your case needs a bit of additional care at the moment.

It can be an issue when generating a huge PDF with watermark, cannot it?

I don't think so. The workaround I proposed above is the most simple one but there are at least 2 other ways (overriding document renderer, adding page event listener) to achieve similar goal on the fly, without having to keep everything in memory - those ways just require a bit more code and have other tradeoffs, e.g. for tagged PDF.

for this PR ) Will you accept it? Otherwise I can close it. As I already wrote I believe it's an improvement. It prevents NPE as in my case

If the field wasn't null in your case you would get another exception down the line, right? I think you mentioned that you are getting Invalid PDF one. That does not seem like an improvement for your case to be honest, and that problem would be much harder to debug rather than NPE.

but you logic rely on not null field. Therefore, it should be ensured with the final word.

The logic implies that the field becomes null when the object is flushed on disk, so that the map field is collected by GC.
Memory aside, if we make sure the field is never null and put is called after the object is flushed on disk, this will not have any effect on the resultant PDF (so the call succeeds but is not reflected on the result) which in my opinion is much worse than having an exception we have now because then a user is going to have a bug which will be much harder to debug.

So what could be improved I guess is exception itself - we could add some validation on whether the object is flushed or not (object is flushed <=> map is null) before trying to perform any operations on the map.
This is something we did not do on purpose at the time because even branch prediction and consequent final evaluation still consumes CPU and as it's very low level functionality, overall impact on the performance can be significant. But we can revisit the discussion of course :)

All in all, we are happy to continue to collaborate on this PR but I think in its current shape, having the case you attached in mind, this change is not something that we would be able to merge in the codebase for the reasons I mentioned above.

@arnosthavelka
Copy link
Author

I see your point, but I don't fully agree . The flushing is done by iText (out of my hands). Therefore, it's iText's responsibility to behave correctly -> at least throw an exception when I try to get already flushed page. This way it should be clearer and avoid the confusion.

However, this PR is not solving the originally targeted issue. It's not a complex fix. So, let's close it.

@arnosthavelka arnosthavelka deleted the 21-fix-npe-on-map branch November 25, 2021 07:33
@arnosthavelka
Copy link
Author

Hello @Snipx,

I have one more question or maybe bug. It's related to disabling of the immediate flush feature recommended by you to make the PDF generation stable. See your hint new Document(new PdfDocument(pdfWriter), PageSize.A4, false)).

However, this flag changes the transparency behavior of the watermark. With the disabled immediate flush the watermark transparency (set by setExtGState(new PdfExtGState().setFillOpacity(0.5f))) simply doesn't work. When I enable the immediate flush than it works.

Is it a bug or there's some other configuration to make it working?

@Snipx
Copy link
Contributor

Snipx commented Jan 19, 2022

Hey @arnosthavelka, I've taken a look at your question and generally speaking the reason of the issue is again in mixing high-level object layout and low-level content stream manipulation. You are adding instructions to the content stream and expect that the content is placed onto the content stream with showTextAligned call but it's not the case due to immediate flush = false.

One way to fix it would be to flush all the document content before adding that content stream external graphic state and flush the layout object created with showTextAligned right away:

PdfDocument pdfDoc = document.getPdfDocument();
        PdfPage pdfPage = pdfDoc.getPage(pageIndex);
        Rectangle pageSize = pdfPage.getPageSizeWithRotation();

        float x = (pageSize.getLeft() + pageSize.getRight()) / 2;
        float y = (pageSize.getTop() + pageSize.getBottom()) / 2;
        document.flush();
        PdfCanvas over = new PdfCanvas(pdfDoc.getPage(pageIndex));
        over.saveState();
        over.setExtGState(graphicState);
        document.showTextAligned(paragraph, x - 50, y, pageIndex, CENTER, TOP, 45);
        document.flush();
        over.restoreState();

But a better and more idiomatic way to show watermark text in transparent color is just using a dedicated setter:

        Paragraph paragraph = new Paragraph("java.lang.NullPointerException: Cannot invoke \"java.util.Map.get(Object)\" because \"this.map\" is null");
        paragraph.setFontColor(ColorConstants.BLACK, 0.5f);

Now you don't need to deal with external graphics state:

    private void addWatermarkToPage(Document document, int pageIndex, Paragraph paragraph) {
        PdfDocument pdfDoc = document.getPdfDocument();
        PdfPage pdfPage = pdfDoc.getPage(pageIndex);
        Rectangle pageSize = pdfPage.getPageSizeWithRotation();

        float x = (pageSize.getLeft() + pageSize.getRight()) / 2;
        float y = (pageSize.getTop() + pageSize.getBottom()) / 2;
        PdfCanvas over = new PdfCanvas(pdfDoc.getPage(pageIndex));
        document.showTextAligned(paragraph, x - 50, y, pageIndex, CENTER, TOP, 45);
    }

Hope it helps. P.S. I suggest using StackOverflow for similar questions as it might be useful for SO community :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants