Fixing JDK-8130493

classic Classic list List threaded Threaded
18 messages Options
Reply | Threaded
Open this post in threaded view
|

Fixing JDK-8130493

Schaef, Martin

Hi,

 

We have some users suffering from 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() and re-thrown as an Abort.

 

This Abort reaches JavaCompiler.processAnnotations() and is finally caught in JavaCompiler.compile() 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?

 

Reply | Threaded
Open this post in threaded view
|

Re: Fixing JDK-8130493

Jonathan Gibbons

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 (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() and re-thrown as an Abort.

 

This Abort reaches JavaCompiler.processAnnotations() and is finally caught in JavaCompiler.compile() 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?

 


Reply | Threaded
Open this post in threaded view
|

Re: Fixing JDK-8130493

Schaef, Martin

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) which already wraps it in a ServiceConfigurationError 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 <[hidden email]>
Date: Tuesday, January 9, 2018 at 1:19 PM
To: "Schaef, Martin" <[hidden email]>, "[hidden email]" <[hidden email]>
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 (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() and re-thrown as an Abort.

 

This Abort reaches JavaCompiler.processAnnotations() and is finally caught in JavaCompiler.compile() 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?

 



Reply | Threaded
Open this post in threaded view
|

Re: Fixing JDK-8130493

Schaef, Martin

Let me revise that:

 

In  JavacProcessingEnvironment.initProcessorIterator the code chooses one of two types of iterators:

The NameProcessIterator if ther -processor flag is given or the ServiceIterator.

 

If we look at the NameProcessIterator.hasNext() 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. This behaves different than the NameProcessIterator: hasNext() only checks if a next name exists and would not throw a NoClassDefFoundError. Then next() throws the NoClassDefFoundError which gets caught by the catch-all in JavaProcessingEnvironment.ServiceIterator.next() and wrapped into an Abort.

 

So, to summarize:

For a NameProcessIterator, a missing class causes a AnnotationProcessingError thrown from NameProcessIterator.hasNext() and for ServiceIterator, it causes a Abert thrown by  JavaProcessingEnvironment.ServiceIterator.next().

 

I could fix my problem and JDK-8130493 by checking if the class can be loaded in the hasNext method and changing the exception handling in  JavaProcessingEnvironment.ServiceIterator.next(). 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 <[hidden email]> on behalf of "Schaef, Martin" <[hidden email]>
Date: Tuesday, January 9, 2018 at 3:37 PM
To: Jonathan Gibbons <[hidden email]>, "[hidden email]" <[hidden email]>
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) which already wraps it in a ServiceConfigurationError 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 <[hidden email]>
Date: Tuesday, January 9, 2018 at 1:19 PM
To: "Schaef, Martin" <[hidden email]>, "[hidden email]" <[hidden email]>
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 (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() and re-thrown as an Abort.

 

This Abort reaches JavaCompiler.processAnnotations() and is finally caught in JavaCompiler.compile() 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?

 





jdk-8130493.patch (4K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: Fixing JDK-8130493

Jonathan Gibbons

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 the code chooses one of two types of iterators:

The NameProcessIterator if ther -processor flag is given or the ServiceIterator.

 

If we look at the NameProcessIterator.hasNext() 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. This behaves different than the NameProcessIterator: hasNext() only checks if a next name exists and would not throw a NoClassDefFoundError. Then next() throws the NoClassDefFoundError which gets caught by the catch-all in JavaProcessingEnvironment.ServiceIterator.next() and wrapped into an Abort.

 

So, to summarize:

For a NameProcessIterator, a missing class causes a AnnotationProcessingError thrown from NameProcessIterator.hasNext() and for ServiceIterator, it causes a Abert thrown by  JavaProcessingEnvironment.ServiceIterator.next().

 

I could fix my problem and JDK-8130493 by checking if the class can be loaded in the hasNext method and changing the exception handling in  JavaProcessingEnvironment.ServiceIterator.next(). 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 [hidden email] on behalf of "Schaef, Martin" [hidden email]
Date: Tuesday, January 9, 2018 at 3:37 PM
To: Jonathan Gibbons [hidden email], [hidden email] [hidden email]
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) which already wraps it in a ServiceConfigurationError 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 [hidden email]
Date: Tuesday, January 9, 2018 at 1:19 PM
To: "Schaef, Martin" [hidden email], [hidden email] [hidden email]
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 (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() and re-thrown as an Abort.

 

This Abort reaches JavaCompiler.processAnnotations() and is finally caught in JavaCompiler.compile() 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?

 





Reply | Threaded
Open this post in threaded view
|

Re: Fixing JDK-8130493

Schaef, Martin

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 <[hidden email]>
Date: Wednesday, January 10, 2018 at 11:39 AM
To: "Schaef, Martin" <[hidden email]>, "[hidden email]" <[hidden email]>
Cc: "Hohensee, Paul" <[hidden email]>
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 the code chooses one of two types of iterators:

The NameProcessIterator if ther -processor flag is given or the ServiceIterator.

 

If we look at the NameProcessIterator.hasNext() 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. This behaves different than the NameProcessIterator: hasNext() only checks if a next name exists and would not throw a NoClassDefFoundError. Then next() throws the NoClassDefFoundError which gets caught by the catch-all in JavaProcessingEnvironment.ServiceIterator.next() and wrapped into an Abort.

 

So, to summarize:

For a NameProcessIterator, a missing class causes a AnnotationProcessingError thrown from NameProcessIterator.hasNext() and for ServiceIterator, it causes a Abert thrown by  JavaProcessingEnvironment.ServiceIterator.next().

 

I could fix my problem and JDK-8130493 by checking if the class can be loaded in the hasNext method and changing the exception handling in  JavaProcessingEnvironment.ServiceIterator.next(). 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 [hidden email] on behalf of "Schaef, Martin" [hidden email]
Date: Tuesday, January 9, 2018 at 3:37 PM
To: Jonathan Gibbons [hidden email], [hidden email] [hidden email]
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) which already wraps it in a ServiceConfigurationError 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 [hidden email]
Date: Tuesday, January 9, 2018 at 1:19 PM
To: "Schaef, Martin" [hidden email], [hidden email] [hidden email]
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 (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() and re-thrown as an Abort.

 

This Abort reaches JavaCompiler.processAnnotations() and is finally caught in JavaCompiler.compile() 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?

 







Reply | Threaded
Open this post in threaded view
|

Re: Fixing JDK-8130493

Jonathan Gibbons

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 [hidden email]
Date: Wednesday, January 10, 2018 at 11:39 AM
To: "Schaef, Martin" [hidden email], [hidden email] [hidden email]
Cc: "Hohensee, Paul" [hidden email]
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 the code chooses one of two types of iterators:

The NameProcessIterator if ther -processor flag is given or the ServiceIterator.

 

If we look at the NameProcessIterator.hasNext() 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. This behaves different than the NameProcessIterator: hasNext() only checks if a next name exists and would not throw a NoClassDefFoundError. Then next() throws the NoClassDefFoundError which gets caught by the catch-all in JavaProcessingEnvironment.ServiceIterator.next() and wrapped into an Abort.

 

So, to summarize:

For a NameProcessIterator, a missing class causes a AnnotationProcessingError thrown from NameProcessIterator.hasNext() and for ServiceIterator, it causes a Abert thrown by  JavaProcessingEnvironment.ServiceIterator.next().

 

I could fix my problem and JDK-8130493 by checking if the class can be loaded in the hasNext method and changing the exception handling in  JavaProcessingEnvironment.ServiceIterator.next(). 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 [hidden email] on behalf of "Schaef, Martin" [hidden email]
Date: Tuesday, January 9, 2018 at 3:37 PM
To: Jonathan Gibbons [hidden email], [hidden email] [hidden email]
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) which already wraps it in a ServiceConfigurationError 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 [hidden email]
Date: Tuesday, January 9, 2018 at 1:19 PM
To: "Schaef, Martin" [hidden email], [hidden email] [hidden email]
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 (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() and re-thrown as an Abort.

 

This Abort reaches JavaCompiler.processAnnotations() and is finally caught in JavaCompiler.compile() 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?

 








Reply | Threaded
Open this post in threaded view
|

Re: Fixing JDK-8130493

Schaef, Martin

Mostly because I cannot see if there are other Exceptions that should be wrapped into Abort. I saw other places that throw AnnotationProcessingError, so I thought this would be a good way forward.

 

Do you think that I can safely wrap all exceptions into an Error at this point?

 

If so, I’ll adjust my patch.

 

From: Jonathan Gibbons <[hidden email]>
Date: Wednesday, January 10, 2018 at 12:24 PM
To: "Schaef, Martin" <[hidden email]>, "[hidden email]" <[hidden email]>
Cc: "Hohensee, Paul" <[hidden email]>
Subject: Re: Fixing JDK-8130493

 

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 [hidden email]
Date: Wednesday, January 10, 2018 at 11:39 AM
To: "Schaef, Martin" [hidden email], [hidden email] [hidden email]
Cc: "Hohensee, Paul" [hidden email]
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 the code chooses one of two types of iterators:

The NameProcessIterator if ther -processor flag is given or the ServiceIterator.

 

If we look at the NameProcessIterator.hasNext() 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. This behaves different than the NameProcessIterator: hasNext() only checks if a next name exists and would not throw a NoClassDefFoundError. Then next() throws the NoClassDefFoundError which gets caught by the catch-all in JavaProcessingEnvironment.ServiceIterator.next() and wrapped into an Abort.

 

So, to summarize:

For a NameProcessIterator, a missing class causes a AnnotationProcessingError thrown from NameProcessIterator.hasNext() and for ServiceIterator, it causes a Abert thrown by  JavaProcessingEnvironment.ServiceIterator.next().

 

I could fix my problem and JDK-8130493 by checking if the class can be loaded in the hasNext method and changing the exception handling in  JavaProcessingEnvironment.ServiceIterator.next(). 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 [hidden email] on behalf of "Schaef, Martin" [hidden email]
Date: Tuesday, January 9, 2018 at 3:37 PM
To: Jonathan Gibbons [hidden email], [hidden email] [hidden email]
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) which already wraps it in a ServiceConfigurationError 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 [hidden email]
Date: Tuesday, January 9, 2018 at 1:19 PM
To: "Schaef, Martin" [hidden email], [hidden email] [hidden email]
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 (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() and re-thrown as an Abort.

 

This Abort reaches JavaCompiler.processAnnotations() and is finally caught in JavaCompiler.compile() 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?

 











Reply | Threaded
Open this post in threaded view
|

Re: Fixing JDK-8130493

Jan Lahoda
In reply to this post by Jonathan Gibbons
For a minimal change, I think it should be enough to change (in
ServiceIterator) the existing:
---
             } catch (Throwable t) {
                 throw new Abort(t);
             }
---
to
---
             } catch (Throwable t) {
                 throw new AnnotationProcessingError(t);
             }
---

AnnotationProcessingError is handled both in JavacTaskImpl and
com.sun.tools.javac.main.Main. For command line javac, this would get an
output like:
---
An annotation processor threw an uncaught exception.
Consult the following stack trace for details.
java.lang.ClassFormatError: Truncated class file
[stacktrace]
---

Which is in line with the behavior when -processor <name> is used
(NameProcessIterator).

But I guess it would be good to improve consistency of error handling in
JavacProcessingEnvironment at some point.

Jan

On 10.1.2018 18:23, Jonathan Gibbons wrote:

> 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 <[hidden email]>
>> *Date: *Wednesday, January 10, 2018 at 11:39 AM
>> *To: *"Schaef, Martin" <[hidden email]>,
>> "[hidden email]" <[hidden email]>
>> *Cc: *"Hohensee, Paul" <[hidden email]>
>> *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 <[hidden email]>
>>     <mailto:[hidden email]> on behalf of
>>     "Schaef, Martin" <[hidden email]> <mailto:[hidden email]>
>>     *Date: *Tuesday, January 9, 2018 at 3:37 PM
>>     *To: *Jonathan Gibbons <[hidden email]>
>>     <mailto:[hidden email]>,
>>     "[hidden email]"
>>     <mailto:[hidden email]>
>>     <[hidden email]> <mailto:[hidden email]>
>>     *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 <[hidden email]>
>>     <mailto:[hidden email]>
>>     *Date: *Tuesday, January 9, 2018 at 1:19 PM
>>     *To: *"Schaef, Martin" <[hidden email]>
>>     <mailto:[hidden email]>, "[hidden email]"
>>     <mailto:[hidden email]>
>>     <[hidden email]> <mailto:[hidden email]>
>>     *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?
>>
>>
>>
>>
>>
>>
>>
>
Reply | Threaded
Open this post in threaded view
|

Re: Fixing JDK-8130493

Schaef, Martin
Tested it, and it works for my use case and the example in JDK-8130493. See patch below.
Want to go forward with this?


diff --new-file --unified --recursive --exclude='*.autotools' --exclude='*.cproject' --exclude='*.project' ./langtools/src/share/classes/com/sun/tools/javac/processing/JavacProcessingEnvironment.java ./langtools/src/share/classes/com/sun/tools/javac/processing/JavacProcessingEnvironment.java
--- ./langtools/src/share/classes/com/sun/tools/javac/processing/JavacProcessingEnvironment.java    2017-10-20 20:28:37.000000000 +0000
+++ ./langtools/src/share/classes/com/sun/tools/javac/processing/JavacProcessingEnvironment.java    2018-01-10 16:46:52.798017039 +0000
a   2018-01-10 17:50:25.093338055 +0000
@@ -339,7 +339,7 @@
                 }
             } catch (Throwable t) {
                 log.error("proc.service.problem");
-                throw new Abort(t);
+                throw new AnnotationProcessingError(t);
             }
         }
 
@@ -361,7 +361,7 @@
                 log.error("proc.bad.config.file", sce.getLocalizedMessage());
                 throw new Abort(sce);
             } catch (Throwable t) {
-                throw new Abort(t);
+                throw new AnnotationProcessingError(t);
             }
         }


On 1/10/18, 12:42 PM, "Jan Lahoda" <[hidden email]> wrote:

    For a minimal change, I think it should be enough to change (in
    ServiceIterator) the existing:
    ---
                 } catch (Throwable t) {
                     throw new Abort(t);
                 }
    ---
    to
    ---
                 } catch (Throwable t) {
                     throw new AnnotationProcessingError(t);
                 }
    ---
   
    AnnotationProcessingError is handled both in JavacTaskImpl and
    com.sun.tools.javac.main.Main. For command line javac, this would get an
    output like:
    ---
    An annotation processor threw an uncaught exception.
    Consult the following stack trace for details.
    java.lang.ClassFormatError: Truncated class file
    [stacktrace]
    ---
   
    Which is in line with the behavior when -processor <name> is used
    (NameProcessIterator).
   
    But I guess it would be good to improve consistency of error handling in
    JavacProcessingEnvironment at some point.
   
    Jan
   
    On 10.1.2018 18:23, Jonathan Gibbons wrote:
    > 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 <[hidden email]>
    >> *Date: *Wednesday, January 10, 2018 at 11:39 AM
    >> *To: *"Schaef, Martin" <[hidden email]>,
    >> "[hidden email]" <[hidden email]>
    >> *Cc: *"Hohensee, Paul" <[hidden email]>
    >> *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 <[hidden email]>
    >>     <mailto:[hidden email]> on behalf of
    >>     "Schaef, Martin" <[hidden email]> <mailto:[hidden email]>
    >>     *Date: *Tuesday, January 9, 2018 at 3:37 PM
    >>     *To: *Jonathan Gibbons <[hidden email]>
    >>     <mailto:[hidden email]>,
    >>     "[hidden email]"
    >>     <mailto:[hidden email]>
    >>     <[hidden email]> <mailto:[hidden email]>
    >>     *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 <[hidden email]>
    >>     <mailto:[hidden email]>
    >>     *Date: *Tuesday, January 9, 2018 at 1:19 PM
    >>     *To: *"Schaef, Martin" <[hidden email]>
    >>     <mailto:[hidden email]>, "[hidden email]"
    >>     <mailto:[hidden email]>
    >>     <[hidden email]> <mailto:[hidden email]>
    >>     *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?
    >>
    >>
    >>
    >>
    >>
    >>
    >>
    >
   

Reply | Threaded
Open this post in threaded view
|

Re: Fixing JDK-8130493

Jonathan Gibbons
I would still like to examine this problem further.  I would like to
understand the right solution, and not just go for the minimal solution.

-- Jon

On 01/10/2018 10:27 AM, Schaef, Martin wrote:

> Tested it, and it works for my use case and the example in JDK-8130493. See patch below.
> Want to go forward with this?
>
>
> diff --new-file --unified --recursive --exclude='*.autotools' --exclude='*.cproject' --exclude='*.project' ./langtools/src/share/classes/com/sun/tools/javac/processing/JavacProcessingEnvironment.java ./langtools/src/share/classes/com/sun/tools/javac/processing/JavacProcessingEnvironment.java
> --- ./langtools/src/share/classes/com/sun/tools/javac/processing/JavacProcessingEnvironment.java    2017-10-20 20:28:37.000000000 +0000
> +++ ./langtools/src/share/classes/com/sun/tools/javac/processing/JavacProcessingEnvironment.java    2018-01-10 16:46:52.798017039 +0000
> a   2018-01-10 17:50:25.093338055 +0000
> @@ -339,7 +339,7 @@
>                   }
>               } catch (Throwable t) {
>                   log.error("proc.service.problem");
> -                throw new Abort(t);
> +                throw new AnnotationProcessingError(t);
>               }
>           }
>  
> @@ -361,7 +361,7 @@
>                   log.error("proc.bad.config.file", sce.getLocalizedMessage());
>                   throw new Abort(sce);
>               } catch (Throwable t) {
> -                throw new Abort(t);
> +                throw new AnnotationProcessingError(t);
>               }
>           }
>
>
> On 1/10/18, 12:42 PM, "Jan Lahoda" <[hidden email]> wrote:
>
>      For a minimal change, I think it should be enough to change (in
>      ServiceIterator) the existing:
>      ---
>                   } catch (Throwable t) {
>                       throw new Abort(t);
>                   }
>      ---
>      to
>      ---
>                   } catch (Throwable t) {
>                       throw new AnnotationProcessingError(t);
>                   }
>      ---
>      
>      AnnotationProcessingError is handled both in JavacTaskImpl and
>      com.sun.tools.javac.main.Main. For command line javac, this would get an
>      output like:
>      ---
>      An annotation processor threw an uncaught exception.
>      Consult the following stack trace for details.
>      java.lang.ClassFormatError: Truncated class file
>      [stacktrace]
>      ---
>      
>      Which is in line with the behavior when -processor <name> is used
>      (NameProcessIterator).
>      
>      But I guess it would be good to improve consistency of error handling in
>      JavacProcessingEnvironment at some point.
>      
>      Jan
>      
>      On 10.1.2018 18:23, Jonathan Gibbons wrote:
>      > 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 <[hidden email]>
>      >> *Date: *Wednesday, January 10, 2018 at 11:39 AM
>      >> *To: *"Schaef, Martin" <[hidden email]>,
>      >> "[hidden email]" <[hidden email]>
>      >> *Cc: *"Hohensee, Paul" <[hidden email]>
>      >> *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 <[hidden email]>
>      >>     <mailto:[hidden email]> on behalf of
>      >>     "Schaef, Martin" <[hidden email]> <mailto:[hidden email]>
>      >>     *Date: *Tuesday, January 9, 2018 at 3:37 PM
>      >>     *To: *Jonathan Gibbons <[hidden email]>
>      >>     <mailto:[hidden email]>,
>      >>     "[hidden email]"
>      >>     <mailto:[hidden email]>
>      >>     <[hidden email]> <mailto:[hidden email]>
>      >>     *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 <[hidden email]>
>      >>     <mailto:[hidden email]>
>      >>     *Date: *Tuesday, January 9, 2018 at 1:19 PM
>      >>     *To: *"Schaef, Martin" <[hidden email]>
>      >>     <mailto:[hidden email]>, "[hidden email]"
>      >>     <mailto:[hidden email]>
>      >>     <[hidden email]> <mailto:[hidden email]>
>      >>     *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?
>      >>
>      >>
>      >>
>      >>
>      >>
>      >>
>      >>
>      >
>      
>

Reply | Threaded
Open this post in threaded view
|

Re: Fixing JDK-8130493

Jonathan Gibbons
In reply to this post by Schaef, Martin
This problem just got more complicated.

It looks like you're reporting a problem in an older version of JDK. You reference com.sun.tools.javac.util.ServiceLoader, but that class no longer exists. (At this point in javac, it was a temporary "replacement" for java.util.ServiceLoader.)

So, that doesn't invalidate the general problem report, but it does mean that we are less likely to patch an older version for this issue.

-- Jon

On 01/09/2018 12:36 PM, Schaef, Martin wrote:

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) which already wraps it in a ServiceConfigurationError 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 [hidden email]
Date: Tuesday, January 9, 2018 at 1:19 PM
To: "Schaef, Martin" [hidden email], [hidden email] [hidden email]
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 (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() and re-thrown as an Abort.

 

This Abort reaches JavaCompiler.processAnnotations() and is finally caught in JavaCompiler.compile() 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?

 




Reply | Threaded
Open this post in threaded view
|

Re: Fixing JDK-8130493

Schaef, Martin
In reply to this post by Jonathan Gibbons
My first question would be, what is the use case where iterating through annotation processors should Abort the compiler and exit without error message?
Currently, I can’t think of a case where this is expected.
I would assume that the idea behind the lazy iterator in “hasNext” is that some annotation processors can be discovered but not loaded. But for this case, the “next” method shouldn’t just throw all the way up to Main. Otherwise, you don’t really need lazy loading.

Can you think of a scenario where a processor is discovered but cannot be loaded? What’s the expected behavior?
 

On 1/10/18, 1:32 PM, "Jonathan Gibbons" <[hidden email]> wrote:

    I would still like to examine this problem further.  I would like to
    understand the right solution, and not just go for the minimal solution.
   
    -- Jon
   
    On 01/10/2018 10:27 AM, Schaef, Martin wrote:
    > Tested it, and it works for my use case and the example in JDK-8130493. See patch below.
    > Want to go forward with this?
    >
    >
    > diff --new-file --unified --recursive --exclude='*.autotools' --exclude='*.cproject' --exclude='*.project' ./langtools/src/share/classes/com/sun/tools/javac/processing/JavacProcessingEnvironment.java ./langtools/src/share/classes/com/sun/tools/javac/processing/JavacProcessingEnvironment.java
    > --- ./langtools/src/share/classes/com/sun/tools/javac/processing/JavacProcessingEnvironment.java    2017-10-20 20:28:37.000000000 +0000
    > +++ ./langtools/src/share/classes/com/sun/tools/javac/processing/JavacProcessingEnvironment.java    2018-01-10 16:46:52.798017039 +0000
    > a   2018-01-10 17:50:25.093338055 +0000
    > @@ -339,7 +339,7 @@
    >                   }
    >               } catch (Throwable t) {
    >                   log.error("proc.service.problem");
    > -                throw new Abort(t);
    > +                throw new AnnotationProcessingError(t);
    >               }
    >           }
    >  
    > @@ -361,7 +361,7 @@
    >                   log.error("proc.bad.config.file", sce.getLocalizedMessage());
    >                   throw new Abort(sce);
    >               } catch (Throwable t) {
    > -                throw new Abort(t);
    > +                throw new AnnotationProcessingError(t);
    >               }
    >           }
    >
    >
    > On 1/10/18, 12:42 PM, "Jan Lahoda" <[hidden email]> wrote:
    >
    >      For a minimal change, I think it should be enough to change (in
    >      ServiceIterator) the existing:
    >      ---
    >                   } catch (Throwable t) {
    >                       throw new Abort(t);
    >                   }
    >      ---
    >      to
    >      ---
    >                   } catch (Throwable t) {
    >                       throw new AnnotationProcessingError(t);
    >                   }
    >      ---
    >      
    >      AnnotationProcessingError is handled both in JavacTaskImpl and
    >      com.sun.tools.javac.main.Main. For command line javac, this would get an
    >      output like:
    >      ---
    >      An annotation processor threw an uncaught exception.
    >      Consult the following stack trace for details.
    >      java.lang.ClassFormatError: Truncated class file
    >      [stacktrace]
    >      ---
    >      
    >      Which is in line with the behavior when -processor <name> is used
    >      (NameProcessIterator).
    >      
    >      But I guess it would be good to improve consistency of error handling in
    >      JavacProcessingEnvironment at some point.
    >      
    >      Jan
    >      
    >      On 10.1.2018 18:23, Jonathan Gibbons wrote:
    >      > 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 <[hidden email]>
    >      >> *Date: *Wednesday, January 10, 2018 at 11:39 AM
    >      >> *To: *"Schaef, Martin" <[hidden email]>,
    >      >> "[hidden email]" <[hidden email]>
    >      >> *Cc: *"Hohensee, Paul" <[hidden email]>
    >      >> *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 <[hidden email]>
    >      >>     <mailto:[hidden email]> on behalf of
    >      >>     "Schaef, Martin" <[hidden email]> <mailto:[hidden email]>
    >      >>     *Date: *Tuesday, January 9, 2018 at 3:37 PM
    >      >>     *To: *Jonathan Gibbons <[hidden email]>
    >      >>     <mailto:[hidden email]>,
    >      >>     "[hidden email]"
    >      >>     <mailto:[hidden email]>
    >      >>     <[hidden email]> <mailto:[hidden email]>
    >      >>     *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 <[hidden email]>
    >      >>     <mailto:[hidden email]>
    >      >>     *Date: *Tuesday, January 9, 2018 at 1:19 PM
    >      >>     *To: *"Schaef, Martin" <[hidden email]>
    >      >>     <mailto:[hidden email]>, "[hidden email]"
    >      >>     <mailto:[hidden email]>
    >      >>     <[hidden email]> <mailto:[hidden email]>
    >      >>     *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?
    >      >>
    >      >>
    >      >>
    >      >>
    >      >>
    >      >>
    >      >>
    >      >
    >      
    >
   
   

Reply | Threaded
Open this post in threaded view
|

Re: Fixing JDK-8130493

Jonathan Gibbons
Martin,

Your first question is the key question, although maybe not in the sense you mean.

My first question would be, what is the use case where iterating through annotation processors should Abort the compiler and exit without error message?

Note the emphasis is on "exiting without an error message".  That is the bug that needs to be fixed.   In general, Abort should be used to halt the compilation after reporting an informative error message.  I see a number of use-sites in the JavacProcessingEnvironment class where this pattern is not followed correctly, and that hints at the correct solution.  If an error message is reported, that will (a) not surprise the user that the compiler seems to ignore the issue , and (b) will cause a non-zero exit code from javac.

-- Jon




On 01/10/2018 10:43 AM, Schaef, Martin wrote:
My first question would be, what is the use case where iterating through annotation processors should Abort the compiler and exit without error message?
Currently, I can’t think of a case where this is expected. 
I would assume that the idea behind the lazy iterator in “hasNext” is that some annotation processors can be discovered but not loaded. But for this case, the “next” method shouldn’t just throw all the way up to Main. Otherwise, you don’t really need lazy loading.

Can you think of a scenario where a processor is discovered but cannot be loaded? What’s the expected behavior?
 

On 1/10/18, 1:32 PM, "Jonathan Gibbons" [hidden email] wrote:

    I would still like to examine this problem further.  I would like to 
    understand the right solution, and not just go for the minimal solution.
    
    -- Jon
    
    On 01/10/2018 10:27 AM, Schaef, Martin wrote:
    > Tested it, and it works for my use case and the example in JDK-8130493. See patch below.
    > Want to go forward with this?
    >
    >
    > diff --new-file --unified --recursive --exclude='*.autotools' --exclude='*.cproject' --exclude='*.project' ./langtools/src/share/classes/com/sun/tools/javac/processing/JavacProcessingEnvironment.java ./langtools/src/share/classes/com/sun/tools/javac/processing/JavacProcessingEnvironment.java
    > --- ./langtools/src/share/classes/com/sun/tools/javac/processing/JavacProcessingEnvironment.java    2017-10-20 20:28:37.000000000 +0000
    > +++ ./langtools/src/share/classes/com/sun/tools/javac/processing/JavacProcessingEnvironment.java    2018-01-10 16:46:52.798017039 +0000
    > a   2018-01-10 17:50:25.093338055 +0000
    > @@ -339,7 +339,7 @@
    >                   }
    >               } catch (Throwable t) {
    >                   log.error("proc.service.problem");
    > -                throw new Abort(t);
    > +                throw new AnnotationProcessingError(t);
    >               }
    >           }
    >   
    > @@ -361,7 +361,7 @@
    >                   log.error("proc.bad.config.file", sce.getLocalizedMessage());
    >                   throw new Abort(sce);
    >               } catch (Throwable t) {
    > -                throw new Abort(t);
    > +                throw new AnnotationProcessingError(t);
    >               }
    >           }
    >
    >
    > On 1/10/18, 12:42 PM, "Jan Lahoda" [hidden email] wrote:
    >
    >      For a minimal change, I think it should be enough to change (in
    >      ServiceIterator) the existing:
    >      ---
    >                   } catch (Throwable t) {
    >                       throw new Abort(t);
    >                   }
    >      ---
    >      to
    >      ---
    >                   } catch (Throwable t) {
    >                       throw new AnnotationProcessingError(t);
    >                   }
    >      ---
    >      
    >      AnnotationProcessingError is handled both in JavacTaskImpl and
    >      com.sun.tools.javac.main.Main. For command line javac, this would get an
    >      output like:
    >      ---
    >      An annotation processor threw an uncaught exception.
    >      Consult the following stack trace for details.
    >      java.lang.ClassFormatError: Truncated class file
    >      [stacktrace]
    >      ---
    >      
    >      Which is in line with the behavior when -processor <name> is used
    >      (NameProcessIterator).
    >      
    >      But I guess it would be good to improve consistency of error handling in
    >      JavacProcessingEnvironment at some point.
    >      
    >      Jan
    >      
    >      On 10.1.2018 18:23, Jonathan Gibbons wrote:
    >      > 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 [hidden email]
    >      >> *Date: *Wednesday, January 10, 2018 at 11:39 AM
    >      >> *To: *"Schaef, Martin" [hidden email],
    >      >> [hidden email] [hidden email]
    >      >> *Cc: *"Hohensee, Paul" [hidden email]
    >      >> *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 [hidden email]
    >      >>     [hidden email] on behalf of
    >      >>     "Schaef, Martin" [hidden email] [hidden email]
    >      >>     *Date: *Tuesday, January 9, 2018 at 3:37 PM
    >      >>     *To: *Jonathan Gibbons [hidden email]
    >      >>     [hidden email],
    >      >>     [hidden email]
    >      >>     [hidden email]
    >      >>     [hidden email] [hidden email]
    >      >>     *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 [hidden email]
    >      >>     [hidden email]
    >      >>     *Date: *Tuesday, January 9, 2018 at 1:19 PM
    >      >>     *To: *"Schaef, Martin" [hidden email]
    >      >>     [hidden email], [hidden email]
    >      >>     [hidden email]
    >      >>     [hidden email] [hidden email]
    >      >>     *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?
    >      >>
    >      >>
    >      >>
    >      >>
    >      >>
    >      >>
    >      >>
    >      >
    >      
    >
    
    


Reply | Threaded
Open this post in threaded view
|

Re: Fixing JDK-8130493

Jonathan Gibbons
In reply to this post by Jan Lahoda
Jan,

javac should not generate a stacktrace in this situation. This case is
about a bad class file, and so javac should generate an informative
message, in the family of "bad class file" or "class not found" messages.

javac should only resort to stacktraces when code has crashed or
otherwise behaved unexpectedly, such that javac cannot provide a helpful
message.  There are two sub-cases here ... the crash is in javac code or
libraries that it might call, in which case the message is, "oops, our
fault, please report a bug", or the crash is in user code, in which
case, the message is effectively, "it's your problem , not javac"

-- Jon

On 01/10/2018 09:41 AM, Jan Lahoda wrote:

> For a minimal change, I think it should be enough to change (in
> ServiceIterator) the existing:
> ---
>             } catch (Throwable t) {
>                 throw new Abort(t);
>             }
> ---
> to
> ---
>             } catch (Throwable t) {
>                 throw new AnnotationProcessingError(t);
>             }
> ---
>
> AnnotationProcessingError is handled both in JavacTaskImpl and
> com.sun.tools.javac.main.Main. For command line javac, this would get
> an output like:
> ---
> An annotation processor threw an uncaught exception.
> Consult the following stack trace for details.
> java.lang.ClassFormatError: Truncated class file
> [stacktrace]
> ---
>
> Which is in line with the behavior when -processor <name> is used
> (NameProcessIterator).
>
> But I guess it would be good to improve consistency of error handling
> in JavacProcessingEnvironment at some point.
>
> Jan
>
> On 10.1.2018 18:23, Jonathan Gibbons wrote:
>> 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 <[hidden email]>
>>> *Date: *Wednesday, January 10, 2018 at 11:39 AM
>>> *To: *"Schaef, Martin" <[hidden email]>,
>>> "[hidden email]" <[hidden email]>
>>> *Cc: *"Hohensee, Paul" <[hidden email]>
>>> *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 <[hidden email]>
>>>     <mailto:[hidden email]> on behalf of
>>>     "Schaef, Martin" <[hidden email]> <mailto:[hidden email]>
>>>     *Date: *Tuesday, January 9, 2018 at 3:37 PM
>>>     *To: *Jonathan Gibbons <[hidden email]>
>>>     <mailto:[hidden email]>,
>>>     "[hidden email]"
>>>     <mailto:[hidden email]>
>>>     <[hidden email]>
>>> <mailto:[hidden email]>
>>>     *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 <[hidden email]>
>>>     <mailto:[hidden email]>
>>>     *Date: *Tuesday, January 9, 2018 at 1:19 PM
>>>     *To: *"Schaef, Martin" <[hidden email]>
>>>     <mailto:[hidden email]>, "[hidden email]"
>>>     <mailto:[hidden email]>
>>>     <[hidden email]>
>>> <mailto:[hidden email]>
>>>     *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?
>>>
>>>
>>>
>>>
>>>
>>>
>>>
>>

Reply | Threaded
Open this post in threaded view
|

Re: Fixing JDK-8130493

Jan Lahoda
On 10.1.2018 20:06, Jonathan Gibbons wrote:

> Jan,
>
> javac should not generate a stacktrace in this situation. This case is
> about a bad class file, and so javac should generate an informative
> message, in the family of "bad class file" or "class not found" messages.
>
> javac should only resort to stacktraces when code has crashed or
> otherwise behaved unexpectedly, such that javac cannot provide a helpful
> message.  There are two sub-cases here ... the crash is in javac code or
> libraries that it might call, in which case the message is, "oops, our
> fault, please report a bug", or the crash is in user code, in which
> case, the message is effectively, "it's your problem , not javac"

(Especially for the exception from JDK-8130493.) I think this case is
close to the "it's your problem , not javac" - the problem happens when
the ServiceLoader tries to load a user's class into the VM (putting
aside that it wasn't wrapped in the ServiceConfigurationError, as I'd
expect based on the ServiceLoader javadoc) and it fails. So, while the
specific stacktrace in the bug does not seem particularly useful, in
other cases the exception may be useful for the user to diagnose the
problem. I'd assume that this situation is fairly rare, and so I
personally wouldn't mind more detailed output that could help to
determine what is the cause of the problem.

Jan

>
> -- Jon
>
> On 01/10/2018 09:41 AM, Jan Lahoda wrote:
>> For a minimal change, I think it should be enough to change (in
>> ServiceIterator) the existing:
>> ---
>>             } catch (Throwable t) {
>>                 throw new Abort(t);
>>             }
>> ---
>> to
>> ---
>>             } catch (Throwable t) {
>>                 throw new AnnotationProcessingError(t);
>>             }
>> ---
>>
>> AnnotationProcessingError is handled both in JavacTaskImpl and
>> com.sun.tools.javac.main.Main. For command line javac, this would get
>> an output like:
>> ---
>> An annotation processor threw an uncaught exception.
>> Consult the following stack trace for details.
>> java.lang.ClassFormatError: Truncated class file
>> [stacktrace]
>> ---
>>
>> Which is in line with the behavior when -processor <name> is used
>> (NameProcessIterator).
>>
>> But I guess it would be good to improve consistency of error handling
>> in JavacProcessingEnvironment at some point.
>>
>> Jan
>>
>> On 10.1.2018 18:23, Jonathan Gibbons wrote:
>>> 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 <[hidden email]>
>>>> *Date: *Wednesday, January 10, 2018 at 11:39 AM
>>>> *To: *"Schaef, Martin" <[hidden email]>,
>>>> "[hidden email]" <[hidden email]>
>>>> *Cc: *"Hohensee, Paul" <[hidden email]>
>>>> *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 <[hidden email]>
>>>>     <mailto:[hidden email]> on behalf of
>>>>     "Schaef, Martin" <[hidden email]> <mailto:[hidden email]>
>>>>     *Date: *Tuesday, January 9, 2018 at 3:37 PM
>>>>     *To: *Jonathan Gibbons <[hidden email]>
>>>>     <mailto:[hidden email]>,
>>>>     "[hidden email]"
>>>>     <mailto:[hidden email]>
>>>>     <[hidden email]>
>>>> <mailto:[hidden email]>
>>>>     *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 <[hidden email]>
>>>>     <mailto:[hidden email]>
>>>>     *Date: *Tuesday, January 9, 2018 at 1:19 PM
>>>>     *To: *"Schaef, Martin" <[hidden email]>
>>>>     <mailto:[hidden email]>, "[hidden email]"
>>>>     <mailto:[hidden email]>
>>>>     <[hidden email]>
>>>> <mailto:[hidden email]>
>>>>     *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?
>>>>
>>>>
>>>>
>>>>
>>>>
>>>>
>>>>
>>>
>
Reply | Threaded
Open this post in threaded view
|

Re: Fixing JDK-8130493

Jonathan Gibbons
In this case, it's definitely the user's problem, because it's a bad
class file being provided, but I don't think there's any merit in
providing a stacktrace. It's not like there's an unexpected error such
that it helps us or the user know where in the code the problem
occurred.  A clear message about the offending class (file) should be
sufficient.    This may mean we need catch blocks for recognized
exceptions as well as better handling of Throwable.

It's a strongly-related issue that there are instances of "throw new
Abort" without a message.

-- Jon

On 01/10/2018 12:25 PM, Jan Lahoda wrote:

> On 10.1.2018 20:06, Jonathan Gibbons wrote:
>> Jan,
>>
>> javac should not generate a stacktrace in this situation. This case is
>> about a bad class file, and so javac should generate an informative
>> message, in the family of "bad class file" or "class not found"
>> messages.
>>
>> javac should only resort to stacktraces when code has crashed or
>> otherwise behaved unexpectedly, such that javac cannot provide a helpful
>> message.  There are two sub-cases here ... the crash is in javac code or
>> libraries that it might call, in which case the message is, "oops, our
>> fault, please report a bug", or the crash is in user code, in which
>> case, the message is effectively, "it's your problem , not javac"
>
> (Especially for the exception from JDK-8130493.) I think this case is
> close to the "it's your problem , not javac" - the problem happens
> when the ServiceLoader tries to load a user's class into the VM
> (putting aside that it wasn't wrapped in the
> ServiceConfigurationError, as I'd expect based on the ServiceLoader
> javadoc) and it fails. So, while the specific stacktrace in the bug
> does not seem particularly useful, in other cases the exception may be
> useful for the user to diagnose the problem. I'd assume that this
> situation is fairly rare, and so I personally wouldn't mind more
> detailed output that could help to determine what is the cause of the
> problem.
>
> Jan
>
>>
>> -- Jon
>>
>> On 01/10/2018 09:41 AM, Jan Lahoda wrote:
>>> For a minimal change, I think it should be enough to change (in
>>> ServiceIterator) the existing:
>>> ---
>>>             } catch (Throwable t) {
>>>                 throw new Abort(t);
>>>             }
>>> ---
>>> to
>>> ---
>>>             } catch (Throwable t) {
>>>                 throw new AnnotationProcessingError(t);
>>>             }
>>> ---
>>>
>>> AnnotationProcessingError is handled both in JavacTaskImpl and
>>> com.sun.tools.javac.main.Main. For command line javac, this would get
>>> an output like:
>>> ---
>>> An annotation processor threw an uncaught exception.
>>> Consult the following stack trace for details.
>>> java.lang.ClassFormatError: Truncated class file
>>> [stacktrace]
>>> ---
>>>
>>> Which is in line with the behavior when -processor <name> is used
>>> (NameProcessIterator).
>>>
>>> But I guess it would be good to improve consistency of error handling
>>> in JavacProcessingEnvironment at some point.
>>>
>>> Jan
>>>
>>> On 10.1.2018 18:23, Jonathan Gibbons wrote:
>>>> 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 <[hidden email]>
>>>>> *Date: *Wednesday, January 10, 2018 at 11:39 AM
>>>>> *To: *"Schaef, Martin" <[hidden email]>,
>>>>> "[hidden email]" <[hidden email]>
>>>>> *Cc: *"Hohensee, Paul" <[hidden email]>
>>>>> *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 <[hidden email]>
>>>>>     <mailto:[hidden email]> on behalf of
>>>>>     "Schaef, Martin" <[hidden email]> <mailto:[hidden email]>
>>>>>     *Date: *Tuesday, January 9, 2018 at 3:37 PM
>>>>>     *To: *Jonathan Gibbons <[hidden email]>
>>>>>     <mailto:[hidden email]>,
>>>>>     "[hidden email]"
>>>>>     <mailto:[hidden email]>
>>>>>     <[hidden email]>
>>>>> <mailto:[hidden email]>
>>>>>     *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 <[hidden email]>
>>>>>     <mailto:[hidden email]>
>>>>>     *Date: *Tuesday, January 9, 2018 at 1:19 PM
>>>>>     *To: *"Schaef, Martin" <[hidden email]>
>>>>>     <mailto:[hidden email]>, "[hidden email]"
>>>>>     <mailto:[hidden email]>
>>>>>     <[hidden email]>
>>>>> <mailto:[hidden email]>
>>>>>     *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?
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>>>
>>

Reply | Threaded
Open this post in threaded view
|

Re: Fixing JDK-8130493

Schaef, Martin
Fwiw, in the old code, Main already catches AnnotationProcessingErrors:
http://hg.openjdk.java.net/jdk8u/jdk8u/langtools/file/989188d1a978/src/share/classes/com/sun/tools/javac/main/Main.java#l551
But they don’t make it there because of the issue we discussed that Errors get swallowed and turned into Aborts which get swallowed here:
http://hg.openjdk.java.net/jdk8u/jdk8u/langtools/file/989188d1a978/src/share/classes/com/sun/tools/javac/main/JavaCompiler.java#l863
However, AnnotationProcessingErrors do print a stacktrace, so its not perfect either.

On 1/10/18, 5:43 PM, "Jonathan Gibbons" <[hidden email]> wrote:

    In this case, it's definitely the user's problem, because it's a bad
    class file being provided, but I don't think there's any merit in
    providing a stacktrace. It's not like there's an unexpected error such
    that it helps us or the user know where in the code the problem
    occurred.  A clear message about the offending class (file) should be
    sufficient.    This may mean we need catch blocks for recognized
    exceptions as well as better handling of Throwable.
   
    It's a strongly-related issue that there are instances of "throw new
    Abort" without a message.
   
    -- Jon
   
    On 01/10/2018 12:25 PM, Jan Lahoda wrote:
    > On 10.1.2018 20:06, Jonathan Gibbons wrote:
    >> Jan,
    >>
    >> javac should not generate a stacktrace in this situation. This case is
    >> about a bad class file, and so javac should generate an informative
    >> message, in the family of "bad class file" or "class not found"
    >> messages.
    >>
    >> javac should only resort to stacktraces when code has crashed or
    >> otherwise behaved unexpectedly, such that javac cannot provide a helpful
    >> message.  There are two sub-cases here ... the crash is in javac code or
    >> libraries that it might call, in which case the message is, "oops, our
    >> fault, please report a bug", or the crash is in user code, in which
    >> case, the message is effectively, "it's your problem , not javac"
    >
    > (Especially for the exception from JDK-8130493.) I think this case is
    > close to the "it's your problem , not javac" - the problem happens
    > when the ServiceLoader tries to load a user's class into the VM
    > (putting aside that it wasn't wrapped in the
    > ServiceConfigurationError, as I'd expect based on the ServiceLoader
    > javadoc) and it fails. So, while the specific stacktrace in the bug
    > does not seem particularly useful, in other cases the exception may be
    > useful for the user to diagnose the problem. I'd assume that this
    > situation is fairly rare, and so I personally wouldn't mind more
    > detailed output that could help to determine what is the cause of the
    > problem.
    >
    > Jan
    >
    >>
    >> -- Jon
    >>
    >> On 01/10/2018 09:41 AM, Jan Lahoda wrote:
    >>> For a minimal change, I think it should be enough to change (in
    >>> ServiceIterator) the existing:
    >>> ---
    >>>             } catch (Throwable t) {
    >>>                 throw new Abort(t);
    >>>             }
    >>> ---
    >>> to
    >>> ---
    >>>             } catch (Throwable t) {
    >>>                 throw new AnnotationProcessingError(t);
    >>>             }
    >>> ---
    >>>
    >>> AnnotationProcessingError is handled both in JavacTaskImpl and
    >>> com.sun.tools.javac.main.Main. For command line javac, this would get
    >>> an output like:
    >>> ---
    >>> An annotation processor threw an uncaught exception.
    >>> Consult the following stack trace for details.
    >>> java.lang.ClassFormatError: Truncated class file
    >>> [stacktrace]
    >>> ---
    >>>
    >>> Which is in line with the behavior when -processor <name> is used
    >>> (NameProcessIterator).
    >>>
    >>> But I guess it would be good to improve consistency of error handling
    >>> in JavacProcessingEnvironment at some point.
    >>>
    >>> Jan
    >>>
    >>> On 10.1.2018 18:23, Jonathan Gibbons wrote:
    >>>> 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 <[hidden email]>
    >>>>> *Date: *Wednesday, January 10, 2018 at 11:39 AM
    >>>>> *To: *"Schaef, Martin" <[hidden email]>,
    >>>>> "[hidden email]" <[hidden email]>
    >>>>> *Cc: *"Hohensee, Paul" <[hidden email]>
    >>>>> *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 <[hidden email]>
    >>>>>     <mailto:[hidden email]> on behalf of
    >>>>>     "Schaef, Martin" <[hidden email]> <mailto:[hidden email]>
    >>>>>     *Date: *Tuesday, January 9, 2018 at 3:37 PM
    >>>>>     *To: *Jonathan Gibbons <[hidden email]>
    >>>>>     <mailto:[hidden email]>,
    >>>>>     "[hidden email]"
    >>>>>     <mailto:[hidden email]>
    >>>>>     <[hidden email]>
    >>>>> <mailto:[hidden email]>
    >>>>>     *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 <[hidden email]>
    >>>>>     <mailto:[hidden email]>
    >>>>>     *Date: *Tuesday, January 9, 2018 at 1:19 PM
    >>>>>     *To: *"Schaef, Martin" <[hidden email]>
    >>>>>     <mailto:[hidden email]>, "[hidden email]"
    >>>>>     <mailto:[hidden email]>
    >>>>>     <[hidden email]>
    >>>>> <mailto:[hidden email]>
    >>>>>     *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?
    >>>>>
    >>>>>
    >>>>>
    >>>>>
    >>>>>
    >>>>>
    >>>>>
    >>>>
    >>