Fixing JDK-8130493
Jonathan Gibbons
jonathan.gibbons at oracle.com
Wed Jan 10 17:23:38 UTC 2018
Why would you not just change
from
} catch (Throwable t) {
log.error("proc.service.problem");
throw new Abort(t);
to
} catch (Throwable t) {
log.error("proc.service.problem");
throw new AnnotationProcessingError(t);
and then briefly, concisely, handle AnnotationProcessingError in
JavaCompiler.
-- Jon
On 1/10/18 9:13 AM, Schaef, Martin wrote:
>
> I understand. The bare minimum that fixes the test cases would be to
> change the exception handling in the constructor of ServiceIterator:
>
> http://hg.openjdk.java.net/jdk8u/jdk8u/langtools/file/989188d1a978/src/share/classes/com/sun/tools/javac/processing/JavacProcessingEnvironment.java#l340
>
> and in ServiceIterator.next():
>
> http://hg.openjdk.java.net/jdk8u/jdk8u/langtools/file/989188d1a978/src/share/classes/com/sun/tools/javac/processing/JavacProcessingEnvironment.java#l363
>
> from
>
> } catch (Throwable t) {
>
> log.error("proc.service.problem");
>
> throw new Abort(t);
>
> to
>
> } catch (Exception t) {
>
> log.error("proc.service.problem");
>
> throw new Abort(t);
>
> } catch (Throwable t) {
>
> log.error("Some error text");
>
> throw new AnnotationProcessingError (t);
>
> With this change, the exit code of javac is 3 instead of 0.
>
> Does that sound like something you could support? If so, what should
> go in the log.error messages?
>
> Cheers,
>
> Martin
>
> *From: *Jonathan Gibbons <jonathan.gibbons at oracle.com>
> *Date: *Wednesday, January 10, 2018 at 11:39 AM
> *To: *"Schaef, Martin" <schaef at amazon.com>,
> "compiler-dev at openjdk.java.net" <compiler-dev at openjdk.java.net>
> *Cc: *"Hohensee, Paul" <hohensee at amazon.com>
> *Subject: *Re: Fixing JDK-8130493
>
> Martin,
>
> I have not read your patch, but changing the semantics of the iterator
> sounds like a step too far.
>
> Without trying to implement the following yet, I would expect the
> general direction of a solution to be to one of the following:
>
> 1. if the exception should be fatal, allow it to propagate up to
> JavaCompiler, (similar to as now) but maybe with a custom new wrapper
> exception
>
> 2. if the exception should not be fatal, translate it to an error
> message (log.error(...)) and continue.
>
> It probably needs more analysis and discussion to determine which of
> those two directions to take.
>
> -- Jon
>
> On 1/10/18 8:17 AM, Schaef, Martin wrote:
>
> Let me revise that:
>
> In JavacProcessingEnvironment.initProcessorIterator
> <http://hg.openjdk.java.net/jdk8u/jdk8u/langtools/file/989188d1a978/src/share/classes/com/sun/tools/javac/processing/JavacProcessingEnvironment.java#l256>
> the code chooses one of two types of iterators:
>
> The NameProcessIterator
> <http://hg.openjdk.java.net/jdk8u/jdk8u/langtools/file/989188d1a978/src/share/classes/com/sun/tools/javac/processing/JavacProcessingEnvironment.java#l384>
> if ther -processor flag is given or the ServiceIterator
> <http://hg.openjdk.java.net/jdk8u/jdk8u/langtools/file/989188d1a978/src/share/classes/com/sun/tools/javac/processing/JavacProcessingEnvironment.java#l325>.
>
> If we look at the NameProcessIterator.hasNext()
> <http://hg.openjdk.java.net/jdk8u/jdk8u/langtools/file/989188d1a978/src/share/classes/com/sun/tools/javac/processing/JavacProcessingEnvironment.java#l396>
> implementation, it checks if there is a next name AND checks if
> this class name can be loaded. That is, if the class is not
> present, the hasNext() method already returns false (or, in our
> case, throws the NoClassDefFoundError which gets wrapped into an
> AnnotationProcessingError).
>
> If we look at the ServiceIterator together with the stack trace
> from my previous email, we see that the .hasNext() and .next()
> calls get forwarded into the ServiceLoader.LazyIterator
> <http://hg.openjdk.java.net/jdk8u/jdk8u/langtools/file/989188d1a978/src/share/classes/com/sun/tools/javac/util/ServiceLoader.java#l257>.
> This behaves different than the NameProcessIterator: hasNext()
> <http://hg.openjdk.java.net/jdk8u/jdk8u/langtools/file/989188d1a978/src/share/classes/com/sun/tools/javac/util/ServiceLoader.java#l222>
> only checks if a next name exists and would not throw a
> NoClassDefFoundError. Then next()
> <http://hg.openjdk.java.net/jdk8u/jdk8u/langtools/file/989188d1a978/src/share/classes/com/sun/tools/javac/util/ServiceLoader.java#l256>
> throws the NoClassDefFoundError which gets caught by the catch-all
> in JavaProcessingEnvironment.ServiceIterator.next()
> <http://hg.openjdk.java.net/jdk8u/jdk8u/langtools/file/989188d1a978/src/share/classes/com/sun/tools/javac/processing/JavacProcessingEnvironment.java#l363>
> and wrapped into an Abort.
>
> So, to summarize:
>
> For a NameProcessIterator, a missing class causes a
> AnnotationProcessingError thrown from
> NameProcessIterator.hasNext()
> <http://hg.openjdk.java.net/jdk8u/jdk8u/langtools/file/989188d1a978/src/share/classes/com/sun/tools/javac/processing/JavacProcessingEnvironment.java#l396>
> and for ServiceIterator, it causes a Abert thrown by
> JavaProcessingEnvironment.ServiceIterator.next()
> <http://hg.openjdk.java.net/jdk8u/jdk8u/langtools/file/989188d1a978/src/share/classes/com/sun/tools/javac/processing/JavacProcessingEnvironment.java#l363>.
>
> I could fix my problem and JDK-8130493
> <https://bugs.openjdk.java.net/browse/JDK-8130493> by checking if
> the class can be loaded in the hasNext method and changing the
> exception handling in
> JavaProcessingEnvironment.ServiceIterator.next()
> <http://hg.openjdk.java.net/jdk8u/jdk8u/langtools/file/989188d1a978/src/share/classes/com/sun/tools/javac/processing/JavacProcessingEnvironment.java#l363>.
> The problem with that is that the lazy iterator is not lazy
> anymore. Is there a reason why this Iterator has to be lazy?
>
> I attached a patch that solves the issue for me. Feedback would be
> great.
>
> Cheers,
>
> Martin
>
> *From: *compiler-dev <compiler-dev-bounces at openjdk.java.net>
> <mailto:compiler-dev-bounces at openjdk.java.net> on behalf of
> "Schaef, Martin" <schaef at amazon.com> <mailto:schaef at amazon.com>
> *Date: *Tuesday, January 9, 2018 at 3:37 PM
> *To: *Jonathan Gibbons <jonathan.gibbons at oracle.com>
> <mailto:jonathan.gibbons at oracle.com>,
> "compiler-dev at openjdk.java.net"
> <mailto:compiler-dev at openjdk.java.net>
> <compiler-dev at openjdk.java.net> <mailto:compiler-dev at openjdk.java.net>
> *Subject: *Re: Fixing JDK-8130493
>
> It’s a user class. See full stack trace below.
>
> The problem is that the ClassNotFound is already caught at
> com.sun.tools.javac.util.ServiceLoader$LazyIterator.next(ServiceLoader.java:255)
> <http://hg.openjdk.java.net/jdk8u/jdk8u/langtools/file/989188d1a978/src/share/classes/com/sun/tools/javac/util/ServiceLoader.java#l257>
> which already wraps it in a ServiceConfigurationError
> <http://hg.openjdk.java.net/jdk8u/jdk8u/langtools/file/989188d1a978/src/share/classes/com/sun/tools/javac/util/ServiceLoader.java#l100>
> in the fail method. That is, the exception gets wrapped twice.
> First, LazyIterator.next wraps it in a ServieConfigurationError,
> then ServiceIterator.next wraps it in an Abort.
>
> Is it safe to change both wrappings?
>
> com.sun.tools.javac.util.Abort: java.lang.NoClassDefFoundError:
> com/amazon/coral/util/reflection/AnnotatedManifestAnnotationProcessor
>
> at
> com.sun.tools.javac.processing.JavacProcessingEnvironment$ServiceIterator.next(JavacProcessingEnvironment.java:364)
>
> at
> com.sun.tools.javac.processing.JavacProcessingEnvironment$ServiceIterator.next(JavacProcessingEnvironment.java:325)
>
> at
> com.sun.tools.javac.processing.JavacProcessingEnvironment$DiscoveredProcessors$ProcessorStateIterator.next(JavacProcessingEnvironment.java:597)
>
> at
> com.sun.tools.javac.processing.JavacProcessingEnvironment.discoverAndRunProcs(JavacProcessingEnvironment.java:690)
>
> at
> com.sun.tools.javac.processing.JavacProcessingEnvironment.access$1800(JavacProcessingEnvironment.java:91)
>
> at
> com.sun.tools.javac.processing.JavacProcessingEnvironment$Round.run(JavacProcessingEnvironment.java:1035)
>
> at
> com.sun.tools.javac.processing.JavacProcessingEnvironment.doProcessing(JavacProcessingEnvironment.java:1176)
>
> at
> com.sun.tools.javac.main.JavaCompiler.processAnnotations(JavaCompiler.java:1170)
>
> at
> com.sun.tools.javac.main.JavaCompiler.compile(JavaCompiler.java:856)
>
> at com.sun.tools.javac.main.Main.compile(Main.java:523)
>
> at com.sun.tools.javac.main.Main.compile(Main.java:381)
>
> at com.sun.tools.javac.main.Main.compile(Main.java:370)
>
> at com.sun.tools.javac.main.Main.compile(Main.java:361)
>
> at com.sun.tools.javac.Main.compile(Main.java:56)
>
> at com.sun.tools.javac.Main.main(Main.java:42)
>
> Caused by: java.lang.NoClassDefFoundError:
> com/amazon/coral/util/reflection/AnnotatedManifestAnnotationProcessor
>
> at java.lang.ClassLoader.defineClass1(Native Method)
>
> at java.lang.ClassLoader.defineClass(ClassLoader.java:763)
>
> at
> java.security.SecureClassLoader.defineClass(SecureClassLoader.java:142)
>
> at java.net.URLClassLoader.defineClass(URLClassLoader.java:467)
>
> at java.net.URLClassLoader.access$100(URLClassLoader.java:73)
>
> at java.net.URLClassLoader$1.run(URLClassLoader.java:368)
>
> at java.net.URLClassLoader$1.run(URLClassLoader.java:362)
>
> at java.security.AccessController.doPrivileged(Native Method)
>
> at java.net.URLClassLoader.findClass(URLClassLoader.java:361)
>
> at java.lang.ClassLoader.loadClass(ClassLoader.java:424)
>
> at java.lang.ClassLoader.loadClass(ClassLoader.java:357)
>
> at java.lang.Class.forName0(Native Method)
>
> at java.lang.Class.forName(Class.java:348)
>
> at
> com.sun.tools.javac.util.ServiceLoader$LazyIterator.next(ServiceLoader.java:255)
>
> at
> com.sun.tools.javac.util.ServiceLoader$1.next(ServiceLoader.java:337)
>
> at
> com.sun.tools.javac.processing.JavacProcessingEnvironment$ServiceIterator.next(JavacProcessingEnvironment.java:359)
>
> ... 14 more
>
> Caused by: java.lang.ClassNotFoundException:
> com.amazon.coral.util.reflection.AnnotatedManifestAnnotationProcessor
>
> at java.net.URLClassLoader.findClass(URLClassLoader.java:381)
>
> at java.lang.ClassLoader.loadClass(ClassLoader.java:424)
>
> at java.lang.ClassLoader.loadClass(ClassLoader.java:357)
>
> *From: *Jonathan Gibbons <jonathan.gibbons at oracle.com>
> <mailto:jonathan.gibbons at oracle.com>
> *Date: *Tuesday, January 9, 2018 at 1:19 PM
> *To: *"Schaef, Martin" <schaef at amazon.com>
> <mailto:schaef at amazon.com>, "compiler-dev at openjdk.java.net"
> <mailto:compiler-dev at openjdk.java.net>
> <compiler-dev at openjdk.java.net> <mailto:compiler-dev at openjdk.java.net>
> *Subject: *Re: Fixing JDK-8130493
>
> What is the class triggering the ClassNotFoundException?
>
> If it is a user class not being found, it should be wrapped in a
> ClientCodeException. If it is a javac class not being found, it
> is reasonable to wrap it in an Abort.
>
> -- Jon
>
> On 1/9/18 9:52 AM, Schaef, Martin wrote:
>
> Hi,
>
> We have some users suffering from JDK-8130493
> <https://bugs.openjdk.java.net/browse/JDK-8130493> (their
> builds succeed, but the compiler actually failed). I did some
> digging and the following sequence happens:
>
> A ClassNotFoundException is thrown in
> JavaProcessingEnvironment.ServiceIterator.next()
> <http://hg.openjdk.java.net/jdk8u/jdk8u/langtools/file/989188d1a978/src/share/classes/com/sun/tools/javac/processing/JavacProcessingEnvironment.java#l364>
> and re-thrown as an Abort.
>
> This Abort reaches JavaCompiler.processAnnotations()
> <http://hg.openjdk.java.net/jdk8u/jdk8u/langtools/file/989188d1a978/src/share/classes/com/sun/tools/javac/main/JavaCompiler.java#l1170>
> and is finally caught in JavaCompiler.compile()
> <http://hg.openjdk.java.net/jdk8u/jdk8u/langtools/file/989188d1a978/src/share/classes/com/sun/tools/javac/main/JavaCompiler.java#l856> by
> the following snippet:
>
> try {
>
> initProcessAnnotations(processors);
>
> // These method calls must be chained to avoid
> memory leaks
>
> delegateCompiler =
>
> processAnnotations( //<<<<<< ABORT COMES OUT HERE
>
> enterTrees(stopIfError(CompileState.PARSE,
> parseFiles(sourceFileObjects))),
>
> classnames);
>
> …
>
> } catch (Abort ex) {
>
> if (devVerbose)
>
> ex.printStackTrace(System.err);
>
> } finally {
>
> and swallowed ... but it's printed if XDev is set.
>
> What is a proper way to fix this? Is it correct to wrap all
> exceptions in JavaProcessingEnvironmentServiceIterator into
> aborts? Or would it be better to distinguish different Aborts
> in JavaCompiler.java?
>
>
>
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/compiler-dev/attachments/20180110/26f1c9f4/attachment-0001.html>
More information about the compiler-dev
mailing list