[8u] RFR: 8213734: SAXParser.parse(File, ..) does not close resources when Exception occurs.
Hi, Please review this 8u backport. The OpenJDK 11u patch does not apply cleanly after path-reshuffeling due to a) test and code for jaxp are split in JDK 8 b) Some rejects in comments - copyright and last modified date. I've omitted rejected comments. I can manually add them if that's preferred. See below. Bug: https://bugs.openjdk.java.net/browse/JDK-8213734 webrevs: jdk: http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8213734/jdk8/jdk/01/webrev/ jaxp: http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8213734/jdk8/jaxp/01/webrev... Testing: tier1 on Linux x86_64. javax/xml/jaxp tests. New test on Windows without the fix fails and passes with it. Thanks to Simon Tooke who helped testing this patch on Windows. Rejects: $ cat src/com/sun/org/apache/xerces/internal/impl/XMLEntityManager.java.rej --- XMLEntityManager.java +++ XMLEntityManager.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2009, 2017, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2009, 2018, Oracle and/or its affiliates. All rights reserved. */ /* * Licensed to the Apache Software Foundation (ASF) under one or more @@ -89,7 +89,7 @@ * @author K.Venugopal SUN Microsystems * @author Neeraj Bajaj SUN Microsystems * @author Sunitha Reddy SUN Microsystems - * @LastModified: Oct 2017 + * @LastModified: Nov 2018 */ public class XMLEntityManager implements XMLComponent, XMLEntityResolver { Thoughts? Thanks, Severin
On Mon, 2019-08-26 at 17:54 +0200, Severin Gehwolf wrote:
Hi,
Please review this 8u backport. The OpenJDK 11u patch does not apply cleanly after path-reshuffeling due to a) test and code for jaxp are split in JDK 8 b) Some rejects in comments - copyright and last modified date. I've omitted rejected comments. I can manually add them if that's preferred. See below.
Bug: https://bugs.openjdk.java.net/browse/JDK-8213734 webrevs: jdk: http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8213734/jdk8/jdk/01/webrev/ jaxp: http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8213734/jdk8/jaxp/01/webrev...
Testing: tier1 on Linux x86_64. javax/xml/jaxp tests. New test on Windows without the fix fails and passes with it.
Thanks to Simon Tooke who helped testing this patch on Windows.
Rejects: $ cat src/com/sun/org/apache/xerces/internal/impl/XMLEntityManager.java.rej --- XMLEntityManager.java +++ XMLEntityManager.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2009, 2017, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2009, 2018, Oracle and/or its affiliates. All rights reserved. */ /* * Licensed to the Apache Software Foundation (ASF) under one or more @@ -89,7 +89,7 @@ * @author K.Venugopal SUN Microsystems * @author Neeraj Bajaj SUN Microsystems * @author Sunitha Reddy SUN Microsystems - * @LastModified: Oct 2017 + * @LastModified: Nov 2018 */ public class XMLEntityManager implements XMLComponent, XMLEntityResolver {
Gentle reminder. Thanks, Severin
Thoughts?
Thanks, Severin
Hi! I'd appreciate a review of this one. Thanks in advance! See below. On Tue, 2019-09-03 at 17:50 +0200, Severin Gehwolf wrote:
On Mon, 2019-08-26 at 17:54 +0200, Severin Gehwolf wrote:
Hi,
Please review this 8u backport. The OpenJDK 11u patch does not apply cleanly after path-reshuffeling due to a) test and code for jaxp are split in JDK 8 b) Some rejects in comments - copyright and last modified date. I've omitted rejected comments. I can manually add them if that's preferred. See below.
Bug: https://bugs.openjdk.java.net/browse/JDK-8213734 webrevs: jdk: http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8213734/jdk8/jdk/01/webrev/ jaxp: http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8213734/jdk8/jaxp/01/webrev...
Testing: tier1 on Linux x86_64. javax/xml/jaxp tests. New test on Windows without the fix fails and passes with it.
Thanks to Simon Tooke who helped testing this patch on Windows.
Rejects: $ cat src/com/sun/org/apache/xerces/internal/impl/XMLEntityManager.java.rej --- XMLEntityManager.java +++ XMLEntityManager.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2009, 2017, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2009, 2018, Oracle and/or its affiliates. All rights reserved. */ /* * Licensed to the Apache Software Foundation (ASF) under one or more @@ -89,7 +89,7 @@ * @author K.Venugopal SUN Microsystems * @author Neeraj Bajaj SUN Microsystems * @author Sunitha Reddy SUN Microsystems - * @LastModified: Oct 2017 + * @LastModified: Nov 2018 */ public class XMLEntityManager implements XMLComponent, XMLEntityResolver {
Gentle reminder.
Anyone? It's been a month :( FWIW, I've updated XMLEntityManager to include the copyright update upper bound to 2018. The @LastModified lines don't exist in JDK 8u, so I've omitted that. Latest webrevs: webrevs: jdk: http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8213734/jdk8/jdk/01/webrev/ jaxp: http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8213734/jdk8/jaxp/02/webrev... Thanks, Severin
Hi Severin, The patch looks good to me. Cheers, Mario On Fri, Sep 27, 2019 at 10:38 AM Severin Gehwolf <sgehwolf@redhat.com> wrote:
Hi!
I'd appreciate a review of this one. Thanks in advance! See below.
On Tue, 2019-09-03 at 17:50 +0200, Severin Gehwolf wrote:
On Mon, 2019-08-26 at 17:54 +0200, Severin Gehwolf wrote:
Hi,
Please review this 8u backport. The OpenJDK 11u patch does not apply cleanly after path-reshuffeling due to a) test and code for jaxp are split in JDK 8 b) Some rejects in comments - copyright and last modified date. I've omitted rejected comments. I can manually add them if that's preferred. See below.
Bug: https://bugs.openjdk.java.net/browse/JDK-8213734 webrevs: jdk: http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8213734/jdk8/jdk/01/webrev/ jaxp: http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8213734/jdk8/jaxp/01/webrev...
Testing: tier1 on Linux x86_64. javax/xml/jaxp tests. New test on Windows without the fix fails and passes with it.
Thanks to Simon Tooke who helped testing this patch on Windows.
Rejects: $ cat src/com/sun/org/apache/xerces/internal/impl/XMLEntityManager.java.rej --- XMLEntityManager.java +++ XMLEntityManager.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2009, 2017, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2009, 2018, Oracle and/or its affiliates. All rights reserved. */ /* * Licensed to the Apache Software Foundation (ASF) under one or more @@ -89,7 +89,7 @@ * @author K.Venugopal SUN Microsystems * @author Neeraj Bajaj SUN Microsystems * @author Sunitha Reddy SUN Microsystems - * @LastModified: Oct 2017 + * @LastModified: Nov 2018 */ public class XMLEntityManager implements XMLComponent, XMLEntityResolver {
Gentle reminder.
Anyone? It's been a month :(
FWIW, I've updated XMLEntityManager to include the copyright update upper bound to 2018. The @LastModified lines don't exist in JDK 8u, so I've omitted that. Latest webrevs:
webrevs: jdk: http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8213734/jdk8/jdk/01/webrev/ jaxp: http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8213734/jdk8/jaxp/02/webrev...
Thanks, Severin
-- Mario Torre Associate Manager, Software Engineering Red Hat GmbH <https://www.redhat.com> 9704 A60C B4BE A8B8 0F30 9205 5D7E 4952 3F65 7898
On Fri, 2019-10-11 at 16:47 +0200, Mario Torre wrote:
Hi Severin,
The patch looks good to me.
Thanks for the review, Mario! Cheers, Severin
On Fri, Sep 27, 2019 at 10:38 AM Severin Gehwolf <sgehwolf@redhat.com> wrote:
Hi!
I'd appreciate a review of this one. Thanks in advance! See below.
On Tue, 2019-09-03 at 17:50 +0200, Severin Gehwolf wrote:
On Mon, 2019-08-26 at 17:54 +0200, Severin Gehwolf wrote:
Hi,
Please review this 8u backport. The OpenJDK 11u patch does not apply cleanly after path-reshuffeling due to a) test and code for jaxp are split in JDK 8 b) Some rejects in comments - copyright and last modified date. I've omitted rejected comments. I can manually add them if that's preferred. See below.
Bug: https://bugs.openjdk.java.net/browse/JDK-8213734 webrevs: jdk: http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8213734/jdk8/jdk/01/webrev/ jaxp: http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8213734/jdk8/jaxp/01/webrev...
Testing: tier1 on Linux x86_64. javax/xml/jaxp tests. New test on Windows without the fix fails and passes with it.
Thanks to Simon Tooke who helped testing this patch on Windows.
Rejects: $ cat src/com/sun/org/apache/xerces/internal/impl/XMLEntityManager.java.rej --- XMLEntityManager.java +++ XMLEntityManager.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2009, 2017, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2009, 2018, Oracle and/or its affiliates. All rights reserved. */ /* * Licensed to the Apache Software Foundation (ASF) under one or more @@ -89,7 +89,7 @@ * @author K.Venugopal SUN Microsystems * @author Neeraj Bajaj SUN Microsystems * @author Sunitha Reddy SUN Microsystems - * @LastModified: Oct 2017 + * @LastModified: Nov 2018 */ public class XMLEntityManager implements XMLComponent, XMLEntityResolver {
Gentle reminder.
Anyone? It's been a month :(
FWIW, I've updated XMLEntityManager to include the copyright update upper bound to 2018. The @LastModified lines don't exist in JDK 8u, so I've omitted that. Latest webrevs:
webrevs: jdk: http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8213734/jdk8/jdk/01/webrev/ jaxp: http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8213734/jdk8/jaxp/02/webrev...
Thanks, Severin
On 11/10/2019 15:54, Severin Gehwolf wrote:
On Fri, 2019-10-11 at 16:47 +0200, Mario Torre wrote:
Hi Severin,
The patch looks good to me.
Thanks for the review, Mario!
Cheers, Severin
On Fri, Sep 27, 2019 at 10:38 AM Severin Gehwolf <sgehwolf@redhat.com> wrote:
Hi!
I'd appreciate a review of this one. Thanks in advance! See below.
On Tue, 2019-09-03 at 17:50 +0200, Severin Gehwolf wrote:
On Mon, 2019-08-26 at 17:54 +0200, Severin Gehwolf wrote:
Hi,
Please review this 8u backport. The OpenJDK 11u patch does not apply cleanly after path-reshuffeling due to a) test and code for jaxp are split in JDK 8 b) Some rejects in comments - copyright and last modified date. I've omitted rejected comments. I can manually add them if that's preferred. See below.
Bug: https://bugs.openjdk.java.net/browse/JDK-8213734 webrevs: jdk: http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8213734/jdk8/jdk/01/webrev/ jaxp: http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8213734/jdk8/jaxp/01/webrev...
Testing: tier1 on Linux x86_64. javax/xml/jaxp tests. New test on Windows without the fix fails and passes with it.
Thanks to Simon Tooke who helped testing this patch on Windows.
Rejects: $ cat src/com/sun/org/apache/xerces/internal/impl/XMLEntityManager.java.rej --- XMLEntityManager.java +++ XMLEntityManager.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2009, 2017, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2009, 2018, Oracle and/or its affiliates. All rights reserved. */ /* * Licensed to the Apache Software Foundation (ASF) under one or more @@ -89,7 +89,7 @@ * @author K.Venugopal SUN Microsystems * @author Neeraj Bajaj SUN Microsystems * @author Sunitha Reddy SUN Microsystems - * @LastModified: Oct 2017 + * @LastModified: Nov 2018 */ public class XMLEntityManager implements XMLComponent, XMLEntityResolver {
Gentle reminder.
Anyone? It's been a month :(
FWIW, I've updated XMLEntityManager to include the copyright update upper bound to 2018. The @LastModified lines don't exist in JDK 8u, so I've omitted that. Latest webrevs:
webrevs: jdk: http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8213734/jdk8/jdk/01/webrev/ jaxp: http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8213734/jdk8/jaxp/02/webrev...
Thanks, Severin
The copyright header was going to be my one comment about the JAXP patch, so good to see it fixed in the second revision. The @LastModified lines were added by "8193568: @LastModified tag in license header", an odd fix which is apparently not even viewable. Maybe someone will end up having to backport it just because it becomes such a pain :( With the JDK side, you don't really explain why it was necessary to convert the test from a TestNG one. Could you elaborate please? Thanks, -- Andrew :) Senior Free Java Software Engineer Red Hat, Inc. (http://www.redhat.com) PGP Key: ed25519/0xCFDA0F9B35964222 (hkp://keys.gnupg.net) Fingerprint = 5132 579D D154 0ED2 3E04 C5A0 CFDA 0F9B 3596 4222
Hi Andrew, On Tue, 2020-02-18 at 10:58 +0000, Andrew Hughes wrote:
On 11/10/2019 15:54, Severin Gehwolf wrote:
On Fri, 2019-10-11 at 16:47 +0200, Mario Torre wrote:
Hi Severin,
The patch looks good to me.
Thanks for the review, Mario!
Cheers, Severin
On Fri, Sep 27, 2019 at 10:38 AM Severin Gehwolf <sgehwolf@redhat.com> wrote:
Hi!
I'd appreciate a review of this one. Thanks in advance! See below.
On Tue, 2019-09-03 at 17:50 +0200, Severin Gehwolf wrote:
On Mon, 2019-08-26 at 17:54 +0200, Severin Gehwolf wrote:
Hi,
Please review this 8u backport. The OpenJDK 11u patch does not apply cleanly after path-reshuffeling due to a) test and code for jaxp are split in JDK 8 b) Some rejects in comments - copyright and last modified date. I've omitted rejected comments. I can manually add them if that's preferred. See below.
Bug: https://bugs.openjdk.java.net/browse/JDK-8213734 webrevs: jdk: http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8213734/jdk8/jdk/01/webrev/ jaxp: http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8213734/jdk8/jaxp/01/webrev...
Testing: tier1 on Linux x86_64. javax/xml/jaxp tests. New test on Windows without the fix fails and passes with it.
Thanks to Simon Tooke who helped testing this patch on Windows.
Rejects: $ cat src/com/sun/org/apache/xerces/internal/impl/XMLEntityManager.java.rej --- XMLEntityManager.java +++ XMLEntityManager.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2009, 2017, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2009, 2018, Oracle and/or its affiliates. All rights reserved. */ /* * Licensed to the Apache Software Foundation (ASF) under one or more @@ -89,7 +89,7 @@ * @author K.Venugopal SUN Microsystems * @author Neeraj Bajaj SUN Microsystems * @author Sunitha Reddy SUN Microsystems - * @LastModified: Oct 2017 + * @LastModified: Nov 2018 */ public class XMLEntityManager implements XMLComponent, XMLEntityResolver {
Gentle reminder.
Anyone? It's been a month :(
FWIW, I've updated XMLEntityManager to include the copyright update upper bound to 2018. The @LastModified lines don't exist in JDK 8u, so I've omitted that. Latest webrevs:
webrevs: jdk: http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8213734/jdk8/jdk/01/webrev/ jaxp: http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8213734/jdk8/jaxp/02/webrev...
Thanks, Severin
The copyright header was going to be my one comment about the JAXP patch, so good to see it fixed in the second revision. The @LastModified lines were added by "8193568: @LastModified tag in license header", an odd fix which is apparently not even viewable. Maybe someone will end up having to backport it just because it becomes such a pain :(
With the JDK side, you don't really explain why it was necessary to convert the test from a TestNG one. Could you elaborate please?
That was a long time ago. I don't quite remember, but it appears to have been a test infra issue. For example BasePolicy doesn't seem to be in 8. It came with JDK-8067170 which is a huge patch not really suitable to backport. Is that explanation sufficient? Thanks, Severin
participants (3)
-
Andrew Hughes
-
Mario Torre
-
Severin Gehwolf