JDK-8015656 : (coll) unexpected NPE from removeAll causes JavaFX 8 build failure
  • Type: Bug
  • Component: core-libs
  • Sub-Component: java.util:collections
  • Affected Version: 8
  • Priority: P2
  • Status: Closed
  • Resolution: Won't Fix
  • Submitted: 2013-05-30
  • Updated: 2022-02-01
  • Resolved: 2022-02-01
Related Reports
Duplicate :  
JDK-8016337 - ArrayList.removeAll(null) throws NPE
Relates :  
JDK-4802647 - (coll) NullPointerException not thrown by AbstractCollection.retainAll/removeAll
Relates :  
JDK-8021591 - Additional NPE checks
Relates :  
JDK-8159404 - throw UnsupportedOperationException unconditionally for mutator methods
Description
JavaFX 8 build using Gradle failed with JDK8-b91 for all platforms. 

Below is the partial log.

+ gradle_tasks=all
+ '[' -n '' ']'
+ gradle_options=' --info -PBUILD_CLOSED=true -PBINARY_STUB= -PCOMPILE_WEBKIT=true -PCOMPILE_GSTREAMER=true'
+ GRADLE_OPTS=-DPULL_CLOSED=true
+ gradle --info -PBUILD_CLOSED=true -PBINARY_STUB= -PCOMPILE_WEBKIT=true -PCOMPILE_GSTREAMER=true all
Starting Build
================================================ Start building buildSrc
Evaluating root project 'buildSrc' using build file 'C:\workspace\8-gfx-gradle\label\windows-i586-30\javafx\buildSrc\build.gradle'.
Selected primary tasks 'clean', 'build'
:buildSrc:clean
Task ':clean' has not declared any outputs, assuming that it is out-of-date.
:buildSrc:clean UP-TO-DATE
:buildSrc:generateGrammarSource
Executing task ':generateGrammarSource' due to:
No history is available for task ':generateGrammarSource'.
Starting process 'command 'C:\jdk\jdk1.8.0-b91\bin\java.exe''. Working directory: C:\workspace\8-gfx-gradle\label\windows-i586-30\javafx\buildSrc Command: C:\jdk\jdk1.8.0-b91\bin\java.exe -Dfile.encoding=windows-1252 -cp C:\Documents and Settings\hudson\.gradle\caches\artifacts-23\filestore\org.antlr\antlr-runtime\3.1.3\jar\3154e3dfd5b7466df8f5151a95be70584f74f76c\antlr-runtime-3.1.3.jar;C:\Documents and Settings\hudson\.gradle\caches\artifacts-23\filestore\org.antlr\stringtemplate\3.2\jar\6fe2e3bb57daebd1555494818909f9664376dd6c\stringtemplate-3.2.jar;C:\Documents and Settings\hudson\.gradle\caches\artifacts-23\filestore\org.antlr\antlr\3.1.3\jar\1e18576780eb117e410e7a262453a99b01e55b02\antlr-3.1.3.jar org.antlr.Tool -o C:\workspace\8-gfx-gradle\label\windows-i586-30\javafx\buildSrc\build/generated-src/antlr/com/sun/scenario/effect/compiler C:\workspace\8-gfx-gradle\label\windows-i586-30\javafx\buildSrc\src\main\antlr\JSL.g
An attempt to initialize for well behaving parent process finished.
Successfully started process 'command 'C:\jdk\jdk1.8.0-b91\bin\java.exe''
error(10):  internal error: Can't get property indirectDelegates using method get/isIndirectDelegates from org.antlr.tool.Grammar instance : java.lang.NullPointerException
java.util.Objects.requireNonNull(Objects.java:203)
java.util.ArrayList.removeAll(ArrayList.java:674)
org.antlr.tool.CompositeGrammar.getIndirectDelegates(CompositeGrammar.java:222)
org.antlr.tool.Grammar.getIndirectDelegates(Grammar.java:2620)
sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:57)
sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
java.lang.reflect.Method.invoke(Method.java:491)
org.antlr.stringtemplate.language.ASTExpr.invokeMethod(ASTExpr.java:563)
org.antlr.stringtemplate.language.ASTExpr.rawGetObjectProperty(ASTExpr.java:514)
org.antlr.stringtemplate.language.ASTExpr.getObjectProperty(ASTExpr.java:416)
org.antlr.stringtemplate.language.ActionEvaluator.attribute(ActionEvaluator.java:351)
org.antlr.stringtemplate.language.ActionEvaluator.expr(ActionEvaluator.java:136)
org.antlr.stringtemplate.language.ActionEvaluator.templateApplication(ActionEvaluator.java:216)
org.antlr.stringtemplate.language.ActionEvaluator.expr(ActionEvaluator.java:126)
org.antlr.stringtemplate.language.ActionEvaluator.action(ActionEvaluator.java:84)
org.antlr.stringtemplate.language.ASTExpr.write(ASTExpr.java:148)
org.antlr.stringtemplate.StringTemplate.write(StringTemplate.java:700)
org.antlr.stringtemplate.language.ASTExpr.write(ASTExpr.java:722)
org.antlr.stringtemplate.language.ASTExpr.writeAttribute(ASTExpr.java:659)
org.antlr.stringtemplate.language.ActionEvaluator.action(ActionEvaluator.java:86)
org.antlr.stringtemplate.language.ASTExpr.write(ASTExpr.java:148)
org.antlr.stringtemplate.StringTemplate.write(StringTemplate.java:700)
org.antlr.stringtemplate.language.ASTExpr.write(ASTExpr.java:722)
org.antlr.stringtemplate.language.ASTExpr.writeAttribute(ASTExpr.java:659)
org.antlr.stringtemplate.language.ActionEvaluator.action(ActionEvaluator.java:86)
org.antlr.stringtemplate.language.ASTExpr.write(ASTExpr.java:148)
org.antlr.stringtemplate.StringTemplate.write(StringTemplate.java:700)
org.antlr.stringtemplate.language.ASTExpr.write(ASTExpr.java:722)
org.antlr.stringtemplate.language.ASTExpr.writeAttribute(ASTExpr.java:659)
org.antlr.stringtemplate.language.ActionEvaluator.action(ActionEvaluator.java:86)
org.antlr.stringtemplate.language.ASTExpr.write(ASTExpr.java:148)
org.antlr.stringtemplate.StringTemplate.write(StringTemplate.java:700)
org.antlr.codegen.CodeGenerator.write(CodeGenerator.java:1278)
org.antlr.codegen.Target.genRecognizerFile(Target.java:94)
org.antlr.codegen.CodeGenerator.genRecognizer(CodeGenerator.java:463)
org.antlr.Tool.generateRecognizer(Tool.java:607)
org.antlr.Tool.process(Tool.java:429)
org.antlr.Tool.main(Tool.java:91)
Process 'command 'C:\jdk\jdk1.8.0-b91\bin\java.exe'' finished with exit value 1 (state: FAILED)
:buildSrc:generateGrammarSource FAILED

FAILURE: Build failed with an exception.

* What went wrong:
Execution failed for task ':generateGrammarSource'.
> Process 'command 'C:\jdk\jdk1.8.0-b91\bin\java.exe'' finished with non-zero exit value 1

* Try:
Run with --stacktrace option to get the stack trace. Run with --debug option to get more log output.

BUILD FAILED

Total time: 7.75 secs
Archiving artifacts
Sending e-mails to: hang.vo@oracle.com kevin.rushforth@oracle.com richard.bair@oracle.com david.hill@oracle.com peter.zhelezniakov@oracle.com
Finished: FAILURE
Comments
Re-closing as Won't Fix per comment from Kevin Rushforth above.
01-02-2022
I don't think that "Fixed" is the right resolution for this issue. No JDK changeset was pushed, and the version of antlr in question still gets the reported NPE. I think that this bug should be reopened and closed as "Won't fix" to reflect the actual resolution.
02-08-2014
This resolved via JDK-8021591 thus verified in JDK-8021591
05-12-2013
Resolved via JDK-8021591. Behaviour changes will be documented via release notes.
17-09-2013
From the JCK side it would be more convenient to have an assertion stating that NPE-throwing behavaior should be present and consistent across all the Collection implementations - so we could develop more tests and verify that this behavior is the same for all JavaSE implementations. As a Java developer I personally support this point of view as well - specify and consistently throw NullPointerException.
13-06-2013
So the choices are; 1) Revert JDK-4802647 2) Add a temporary compatibility property switch 3) Let things bake longer (and fix the additional identified cases that currently don't throw NPE). Add a switch if needed. Hearing nothing will result in #3 being the default action. I will proceed bug fixes for the additional cases.
13-06-2013
My survey found that there are additional fixes needed for several classes. Subject#SecureSet IdentityHashMap#EntrySet IdentityHashMap#KeySet Collections.checkedEntrySet() ConcurrentHashMap#KeySet ConcurrentHashMap#Values would still need to be fixed.
07-06-2013
BTW with the fix for 4802647 in place are all of those cases listed in the CCC request fixed, or are there still follow up fixes needed for specific implementation classes?
07-06-2013
I don't disagree that the implementation should consistently match the spec. It is a shame it took us 10 years to try and rectify this however. Given the passage of time and the example of antlr we can not ignore the compatibility aspect of this change, in my opinion. One possibility would be to make the change but add a property to allow the old behaviour. We could do this for Early Access to give time for third-party tools to comply and then remove the property for GA. This obviously requires some work and short-term some ugly code, but gives us a transition path. Alternatively, given the change is out there and we have a workaround for the antlr problem, we simply see if this has further ramifications and only if it turns out to be a problem do we consider putting in the new property.
07-06-2013
The antlr code which triggered this failure is a good example of the latent bug which the pre-4802647 code masked. The code should be checking the result of getIndirectDelegates() for null as getDelegates() may return a non-empty collection which would produce an NPE. Failing to throw an NPE when getDelegates() is empty merely masks the error.
06-06-2013
The motivation to fix the long standing bug JDK4802647 was the reduce the diversity of behaviour among implementations and make the behaviour more consistent. It was seen as desirable to eliminate behaviour which depended upon the state of the target Collection. ie. NPE should be consistently thrown whether the target Collection is empty or not. Not throwing NPE for only when the target collections is empty likely masks an application error since the motivation to call retainAll/removeAll implies that the target Collection will at times be non-empty which would generate an NPE for null param. It might even be argued that the collections returned by Collections.unmodifiableXXX and Collections.emptyXXX should throw NPE for null params before throwing UOE as this would be informative of a usage error. The retainAll/removeAll methods are defined in java.util.Collection. To allow for a diversity of behaviours among Collection implementations or to change the specification for sub-interfaces or specific implementations would reduce the value of the Collection API contract--the behaviour of removeAll/retainAll would be diverse and the results of calling these methods on arbitrary collections would now be unpredictable. The case for removeAll consistently throwing NPE is slightly better than for retainAll because the AbstractSet.removeAll implementation has long had the behaviour of throwing NPE whenever the param is null (it calls size() upon the param). The following is a summary of the behaviours of various removeAll/retainAll today (2013-06-06) in jdk8/tl repo. Further work is required to ensure that NPE is thrown for null param to retainAll/removeAll. AbstractCollection: - After 4802647 retainAll/removeAll throw NPE if collection is empty and passed null ArrayList: - After 4802647 retainAll/removeAll throw NPE if collection is empty and passed null Collections.checkedEntrySet() : - retainAll/removeAll do not throw NPE if collection is empty and passed null Collections.setFromMap(): - depends upon underlying map keySet EnumSet: - After 4802647 retainAll/removeAll throw NPE if collection is empty and passed null Vector: - After 4802647 retainAll/removeAll throw NPE if collection is empty and passed null ConcurrentHashMap#KeySet: ConcurrentHashMap#Values: - retainAll/removeAll do not throw NPE if collection is empty and passed null CopyOnWriteArrayList: - After 8001575 retainAll/removeAll throw NPE if collection is empty and passed null CopyOnWriteArraySet: - After 8001575 retainAll/removeAll throw NPE if collection is empty and passed null Subject#SecureSet: - retainAll/removeAll do not throw NPE if collection is empty and passed null nio.ch.Util#UngrowableSet: - After 4802647 retainAll/removeAll throw NPE if collection is empty and passed null (via use of HashSet) AbstractSet: - removeAll throws NPE if passed null IdentityHashMap#EntrySet: IdentityHashMap#KeySet: - does not throw NPE if collection is empty and passed null ConcurrentSkipListSet: - throws NPE if collection is empty and passed null
06-06-2013
As it turns out, we have a workaround for the FX build. The NPE doesn't seem to be affecting the generated files that antlr produces. The reason that it isn't failing in the FX production builds, which still use ant to build, is that the ant build scripts happen to ignore the error. We checked and the same NPE is happening there as well, but just being ignored. So our short term workaround is to ignore an error when running antlr in our gradle build too. This is not a very satisfying workaround, since it will also mask any real failures that may crop up, but will allow us to proceed without requiring two different JDKs to build. We filed the following FX JIRA to implement the workaround: https://javafx-jira.kenai.com/browse/RT-30955
05-06-2013
While technically the antlr code is incorrect it is impractical to put this back on antlr as we need it fixed now and we probably don't want a custom antlr build. Making the implementation conformant to the spec after 10 years is a risky proposition - in most cases I've seen it is the spec that has been changed due to the compatibility risk. I think we have to rollback 4802647 and re-examine how to tackle this spec/implementation mismatch.
04-06-2013
https://github.com/antlr/antlr3/blob/master/tool/src/main/java/org/antlr/tool/CompositeGrammar.java?source=cc getDirectDelegates() returns null and getIndirectDelegates() fails to check for this result.
30-05-2013
The cases that 4802647 changes involve empty collections. Previously if a collection was empty then passing null to it's removeAll/retainAll methods would not produce the expected NPE. The bug occurred because the implementation only dereferenced the null collection during the iteration of it's elements. Empty collections bypass the iteration. The difficulty with reverting to the old behaviour is that it will also preclude overrides from adding enforcement. CopyOnWriteArrayList added checking like 4802647 in Java 7 and 5028425, if implemented, would behave more like post-4802647 than pre-4802647. I don't think we want to preserve compatibility with pre-4802647.
30-05-2013
There was no CCC for JDK-4802647.
30-05-2013
I was able to come up with a short-term workaround that will let us move forward and use JDK 8-b91 to compile our code using gradle (we are about to introduce a dependency on b91 for the DatePicker control). The workaround is to use an older JDK, in our case JDK 8-b90 to run the grammarGenerator task (that task uses antlr and is hitting this JDK bug with b91), and then use b91 for the rest of the build.
30-05-2013
I ran a couple of simple tests of ArrayList.removeAll and I am not entirely sure that the change for JDK-4802647 has caused this failure. I wasn't able to find a simple case that works on b90 and fails on b91. The only way I can get an NPE at java.util.Objects.requireNonNull(Objects.java:203) is if I pass null (not a list that contains null) to ArrayList.removeAll(Collection) and that throws a different NPE with b90. So the possibility should be considered that a bug elsewhere in the JDK is coincidentally hitting this new code.
30-05-2013
Was there a CCC for JDK-4802647? I'm curious as to if the compatibility impact was explored.
30-05-2013
Since the non-NPE-throwing behavior was present for ten years, it might be sensible to back out the fix for JDK-4802647 and change the spec to match the existing implementation.
30-05-2013
This may have been caused by the fix for JDK-4802647.
30-05-2013
We have verified that with no other changes, our build works fine when compiled with JDK 8-b90 but fails with 8-b91. The failure happens on all platforms.
30-05-2013