RFR: [8218152] javac fails and exits with no error if a bad annotation processor is on the classpath

Steve Groeger GROEGES at uk.ibm.com
Thu May 2 08:41:57 UTC 2019


Hi Andrew, 

Thanks for taking a look at this. 

> Regarding the 8u webrev, the two additions for hasNext() and next() in
> JavacProcessingEnvironment.java seem to have been flipped. Was this
> intentional? Comparing patched jdk8u with the jdk/jdk version:

Yes, this was intentional. When I investigated this issue I did initially 
add the changes to hasNext() in the jdk8u code but that didn't resolve the 
issue. Having then looked at a generated stack trace it showed that the 
code was going thru the next() method and failing there rather than in 
hasNext(). Hence the reason the changes were put in the next() method for 
jdk8u and in the hasNext() for jdk11 and above. Seems the 
JavacProcessingEnvoronment.java class has changed since jdk8.

> The test changes look ok and I assume they pass. 

Yes, the tests do pass when run on my local environment for all releases 
jdk8u, jdk11u and jdk12u.

> Long term, we may want to look at backporting
> https://bugs.openjdk.java.net/browse/JDK-8050429
> to avoid this work for every test.

I agree that we need to back-port this at some point. It gets quite time 
consuming having to change tests that are created on jdk11u+ and need to 
be back-ported to jdk8u. 

Is there anything else I need to do in order to get these back-ports 
merged. If so, please let me know.

Thanks
Steve Groeger
IBM Runtime Technologies
Hursley, Winchester
Tel: (44) 1962 816911  Mobex: 279990  Mobile: 07718 517 129
Fax (44) 1962 816800
Lotus Notes: Steve Groeger/UK/IBM
Internet: groeges at uk.ibm.com

Unless stated otherwise above:
IBM United Kingdom Limited - Registered in England and Wales with number 
741598.
Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 3AU



From:   Andrew John Hughes <gnu.andrew at redhat.com>
To:     jdk-updates-dev at openjdk.java.net, "'jdk8u-dev at openjdk.java.net'" 
<jdk8u-dev at openjdk.java.net>
Date:   01/05/2019 17:00
Subject:        Re: RFR: [8218152] javac fails and exits with no error if 
a bad annotation processor is on the classpath
Sent by:        "jdk-updates-dev" 
<jdk-updates-dev-bounces at openjdk.java.net>



[CCing jdk8u-dev for 8u review]

On 01/05/2019 08:04, Steve Groeger wrote:
> Hi all, 
> 
> The following issue [1] has been raised and the associated webrev [2] 
has 
> been approved and the code merged into jdk/jdk [3]
> I am now requesting this fix to be back-ported to jdk8u, jdk11u and 
> jdk12u. 
> The patch imports and works correctly on jdk8u and jdk12u but not quite 
on 
> jdk8u due to difference in the test framework.
> I have created a separate webrev for the jdk8u changes [4].
> I am therefore requesting someone to review these changes and to sponsor 

> these updates to jdk8u, jdk11u and jdk12u.
> 
> [1] 
https://urldefense.proofpoint.com/v2/url?u=https-3A__bugs.openjdk.java.net_browse_JDK-2D8218152&d=DwIFaQ&c=jf_iaSHvJObTbx-siA1ZOg&r=78GW2OHz7nNTH2dBkTx7-TKh2QCt3JD3zukzeUO8RpA&m=ATbJ1iXicP6W17wnytspWscrkGhzqhW7xjApBdRxGug&s=qupvoIE2b9o6H2gvzu3EdQUSjE0HvDBZeDXycoIlg4w&e=

> [2] 
https://urldefense.proofpoint.com/v2/url?u=http-3A__cr.openjdk.java.net_-7Esgroeger_8218152_jdk_webrev.04_&d=DwIFaQ&c=jf_iaSHvJObTbx-siA1ZOg&r=78GW2OHz7nNTH2dBkTx7-TKh2QCt3JD3zukzeUO8RpA&m=ATbJ1iXicP6W17wnytspWscrkGhzqhW7xjApBdRxGug&s=Lx2f4bK7_yW6MWLgQYj-Y8GzL5NbAH95L7X_ptB9SBk&e=

> [3] 
https://urldefense.proofpoint.com/v2/url?u=http-3A__hg.openjdk.java.net_jdk_jdk_rev_d9208a660094&d=DwIFaQ&c=jf_iaSHvJObTbx-siA1ZOg&r=78GW2OHz7nNTH2dBkTx7-TKh2QCt3JD3zukzeUO8RpA&m=ATbJ1iXicP6W17wnytspWscrkGhzqhW7xjApBdRxGug&s=4I9emNaREfISeASbo-TDQVJ3x6_pTiKFtlFPK8K2hTk&e=

> [4] 
https://urldefense.proofpoint.com/v2/url?u=http-3A__cr.openjdk.java.net_-7Esgroeger_8218152_jdk8u_webrev.04_&d=DwIFaQ&c=jf_iaSHvJObTbx-siA1ZOg&r=78GW2OHz7nNTH2dBkTx7-TKh2QCt3JD3zukzeUO8RpA&m=ATbJ1iXicP6W17wnytspWscrkGhzqhW7xjApBdRxGug&s=ByHXRWAiDbkm9dsfgHchCQR4FMQiZc_ge0LYlRQkYD8&e=

> 
> Thanks
> Steve Groeger
> IBM Runtime Technologies
> Hursley, Winchester
> Tel: (44) 1962 816911  Mobex: 279990  Mobile: 07718 517 129
> Fax (44) 1962 816800
> Lotus Notes: Steve Groeger/UK/IBM
> Internet: groeges at uk.ibm.com
> 
> Unless stated otherwise above:
> IBM United Kingdom Limited - Registered in England and Wales with number 

> 741598.
> Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 
3AU
> Unless stated otherwise above:
> IBM United Kingdom Limited - Registered in England and Wales with number 

> 741598. 
> Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 
3AU
> 

Regarding the 8u webrev, the two additions for hasNext() and next() in
JavacProcessingEnvironment.java seem to have been flipped. Was this
intentional? Comparing patched jdk8u with the jdk/jdk version:

---
src/share/classes/com/sun/tools/javac/processing/JavacProcessingEnvironment.java
   2019-05-01 16:50:40.409545256 +0100
+++
../../jdk/src/jdk.compiler/share/classes/com/sun/tools/javac/processing/JavacProcessingEnvironment.java
    2019-05-01 03:00

         public boolean hasNext() {
             try {
-                return iterator.hasNext();
+                return internalHasNext();
             } catch(ServiceConfigurationError sce) {
-                log.error("proc.bad.config.file",
sce.getLocalizedMessage());
+
log.error(Errors.ProcBadConfigFile(sce.getLocalizedMessage()));
                 throw new Abort(sce);
+            } catch (UnsupportedClassVersionError ucve) {
+
log.error(Errors.ProcCantLoadClass(ucve.getLocalizedMessage()));
+                throw new Abort(ucve);
+            } catch (ClassFormatError cfe) {
+
log.error(Errors.ProcCantLoadClass(cfe.getLocalizedMessage()));
+                throw new Abort(cfe);
             } catch (Throwable t) {
-                log.error("proc.bad.config.file", 
t.getLocalizedMessage());
+
log.error(Errors.ProcBadConfigFile(t.getLocalizedMessage()));
                 throw new Abort(t);
             }
         }

...

         public Processor next() {
             try {
-                return iterator.next();
+                return internalNext();
             } catch (ServiceConfigurationError sce) {
-                log.error("proc.bad.config.file",
sce.getLocalizedMessage());
+
log.error(Errors.ProcBadConfigFile(sce.getLocalizedMessage()));
                 throw new Abort(sce);
-            } catch (UnsupportedClassVersionError ucve) {
-                log.error("proc.cant.load.class",
ucve.getLocalizedMessage());
-                throw new Abort(ucve);
-            } catch (ClassFormatError cfe) {
-                log.error("proc.cant.load.class",
cfe.getLocalizedMessage());
-                throw new Abort(cfe);
             } catch (Throwable t) {
-                log.error("proc.bad.config.file", 
t.getLocalizedMessage());
+
log.error(Errors.ProcBadConfigFile(t.getLocalizedMessage()));
                 throw new Abort(t);
             }
         }

The test changes look ok and I assume they pass. Long term, we may want
to look at backporting 
https://urldefense.proofpoint.com/v2/url?u=https-3A__bugs.openjdk.java.net_browse_JDK-2D8050429&d=DwIFaQ&c=jf_iaSHvJObTbx-siA1ZOg&r=78GW2OHz7nNTH2dBkTx7-TKh2QCt3JD3zukzeUO8RpA&m=ATbJ1iXicP6W17wnytspWscrkGhzqhW7xjApBdRxGug&s=q-BAu0YUQlwQmBEoesgfjRpVLy_wpTd8b1UZ7CZ_Akk&e=

to avoid this work for every test.

Thanks,
-- 
Andrew :)

Senior Free Java Software Engineer
Red Hat, Inc. (
https://urldefense.proofpoint.com/v2/url?u=http-3A__www.redhat.com&d=DwIFaQ&c=jf_iaSHvJObTbx-siA1ZOg&r=78GW2OHz7nNTH2dBkTx7-TKh2QCt3JD3zukzeUO8RpA&m=ATbJ1iXicP6W17wnytspWscrkGhzqhW7xjApBdRxGug&s=IXk7fITakTIJN8xZ0a-S80B5oFOvfv5yzV1wdICi7hA&e=
)

PGP Key: ed25519/0xCFDA0F9B35964222 (hkp://keys.gnupg.net)
Fingerprint = 5132 579D D154 0ED2 3E04  C5A0 CFDA 0F9B 3596 4222
https://urldefense.proofpoint.com/v2/url?u=https-3A__keybase.io_gnu-5Fandrew&d=DwIFaQ&c=jf_iaSHvJObTbx-siA1ZOg&r=78GW2OHz7nNTH2dBkTx7-TKh2QCt3JD3zukzeUO8RpA&m=ATbJ1iXicP6W17wnytspWscrkGhzqhW7xjApBdRxGug&s=iHeIglNN6XbY2FUKnkzs8A3gd7i9W_jO428tjpYD-Q0&e=






Unless stated otherwise above:
IBM United Kingdom Limited - Registered in England and Wales with number 
741598. 
Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 3AU



More information about the jdk-updates-dev mailing list