Re: RFR (JAXP): 8146961: Fix PermGen memory leaks caused by static final Exceptions
Joe, Christoph, Thanks for your reviews! With Best Regards, Aleksej On 18/08/16 08:47, Langer, Christoph wrote: +1, nice fix. -Original Message- From: core-libs-dev [mailto:core-libs-dev-boun...@openjdk.java.net] On Behalf Of Joe Wang Sent: Mittwoch, 17. August 2016 18:30 To: Aleks Efimov Cc: core-libs-dev Subject: Re: RFR (JAXP): 8146961: Fix PermGen memory leaks caused by static final Exceptions Looks good, Aleksej. Thanks for coming up with a well-thought solution to get rid of the static RE field. Best, Joe On 8/17/16, 7:04 AM, Aleks Efimov wrote: Hi Joe, Aleksey, Thank you for reviewing the initial fix. I followed the Joe's suggestion (thanks for that) and removed static 'abort' field completely, the functionality was replaced by throwing the exception of newly added type - AbortException. The new webrev with removed 'abort' can be found here: http://cr.openjdk.java.net/~aefimov/8146961/9/01 The Tomcat reproducer attached to the bug report fails without the fix and passes with the new version of the fix. The JPRT and JCK testing again shows no related jaxp tests failures. Best Regards, Aleksej On 15/08/16 21:09, Joe Wang wrote: Hi Aleksej, I suggest we get rid of the static abort. If RuntimeException happens, check where it happens. The first use case looks suspicious as it just returns if it's an instance of RE. For the 2nd case in DOM error report, let's throw a RuntimeException with the specified error message if error happens, and there's no handler or the handler failed to handle it. (would be better than an empty RE) Best, Joe On 8/15/16, 10:38 AM, Aleks Efimov wrote: Hi, Please, help to review the fix for memory leak [1] in com.sun.org.apache.xerces.internal.dom.DOMNormalizer that is caused by usage of static final exceptions. This problem was already fixed in Apache Xerces project [2] and I would like to backport it to JDK9 Xerces implementation. Webrev with the changes: http://cr.openjdk.java.net/~aefimov/8146961/9/00 The Tomcat reproducer attached to the bug report fails without the fix and passes with the fix. The JPRT and JCK testing shows no jaxp tests failures with the proposed fix. With Best Regards, Aleksej [1] https://bugs.openjdk.java.net/browse/JDK-8146961 [2] https://issues.apache.org/jira/browse/XERCESJ-1667
Re: RFR (JAXP): 8146961: Fix PermGen memory leaks caused by static final Exceptions
Hi Peter,
Thank you for review and suggestion to add super constructor call: will
add it before pushing the changes.
Best Regards,
Aleksej
On 18/08/16 11:08, Peter Levart wrote:
Hi Aleks,
Looks OK, but if AbortException is never inspected for a stack trace,
then it could be constructed without it. This is perhaps unnecessary
if it is not on the hot path, but it is easy to just call the
appropriate super constructor:
public class AbortException extends RuntimeException {
private static final long serialVersionUID =
2608302175475740417L;
/**
* Constructor AbortException
*/
public AbortException() { super(null, null, false, false); }
}
Regards, Peter
On 08/17/2016 04:04 PM, Aleks Efimov wrote:
Hi Joe, Aleksey,
Thank you for reviewing the initial fix.
I followed the Joe's suggestion (thanks for that) and removed static
'abort' field completely, the functionality was replaced by throwing
the exception of newly added type - AbortException. The new webrev
with removed 'abort' can be found here:
http://cr.openjdk.java.net/~aefimov/8146961/9/01
The Tomcat reproducer attached to the bug report fails without the
fix and passes with the new version of the fix.
The JPRT and JCK testing again shows no related jaxp tests failures.
Best Regards,
Aleksej
On 15/08/16 21:09, Joe Wang wrote:
Hi Aleksej,
I suggest we get rid of the static abort. If RuntimeException
happens, check where it happens. The first use case looks suspicious
as it just returns if it's an instance of RE. For the 2nd case in
DOM error report, let's throw a RuntimeException with the specified
error message if error happens, and there's no handler or the
handler failed to handle it. (would be better than an empty RE)
Best,
Joe
On 8/15/16, 10:38 AM, Aleks Efimov wrote:
Hi,
Please, help to review the fix for memory leak [1] in
com.sun.org.apache.xerces.internal.dom.DOMNormalizer that is caused
by usage of static final exceptions.
This problem was already fixed in Apache Xerces project [2] and I
would like to backport it to JDK9 Xerces implementation.
Webrev with the changes:
http://cr.openjdk.java.net/~aefimov/8146961/9/00
The Tomcat reproducer attached to the bug report fails without the
fix and passes with the fix.
The JPRT and JCK testing shows no jaxp tests failures with the
proposed fix.
With Best Regards,
Aleksej
[1] https://bugs.openjdk.java.net/browse/JDK-8146961
[2] https://issues.apache.org/jira/browse/XERCESJ-1667
Re: RFR (JAXP): 8146961: Fix PermGen memory leaks caused by static final Exceptions
Hi Aleks,
Looks OK, but if AbortException is never inspected for a stack trace,
then it could be constructed without it. This is perhaps unnecessary if
it is not on the hot path, but it is easy to just call the appropriate
super constructor:
public class AbortException extends RuntimeException {
private static final long serialVersionUID = 2608302175475740417L;
/**
* Constructor AbortException
*/
public AbortException() { super(null, null, false, false); }
}
Regards, Peter
On 08/17/2016 04:04 PM, Aleks Efimov wrote:
Hi Joe, Aleksey,
Thank you for reviewing the initial fix.
I followed the Joe's suggestion (thanks for that) and removed static
'abort' field completely, the functionality was replaced by throwing
the exception of newly added type - AbortException. The new webrev
with removed 'abort' can be found here:
http://cr.openjdk.java.net/~aefimov/8146961/9/01
The Tomcat reproducer attached to the bug report fails without the fix
and passes with the new version of the fix.
The JPRT and JCK testing again shows no related jaxp tests failures.
Best Regards,
Aleksej
On 15/08/16 21:09, Joe Wang wrote:
Hi Aleksej,
I suggest we get rid of the static abort. If RuntimeException
happens, check where it happens. The first use case looks suspicious
as it just returns if it's an instance of RE. For the 2nd case in DOM
error report, let's throw a RuntimeException with the specified error
message if error happens, and there's no handler or the handler
failed to handle it. (would be better than an empty RE)
Best,
Joe
On 8/15/16, 10:38 AM, Aleks Efimov wrote:
Hi,
Please, help to review the fix for memory leak [1] in
com.sun.org.apache.xerces.internal.dom.DOMNormalizer that is caused
by usage of static final exceptions.
This problem was already fixed in Apache Xerces project [2] and I
would like to backport it to JDK9 Xerces implementation.
Webrev with the changes:
http://cr.openjdk.java.net/~aefimov/8146961/9/00
The Tomcat reproducer attached to the bug report fails without the
fix and passes with the fix.
The JPRT and JCK testing shows no jaxp tests failures with the
proposed fix.
With Best Regards,
Aleksej
[1] https://bugs.openjdk.java.net/browse/JDK-8146961
[2] https://issues.apache.org/jira/browse/XERCESJ-1667
RE: RFR (JAXP): 8146961: Fix PermGen memory leaks caused by static final Exceptions
+1, nice fix. > -Original Message- > From: core-libs-dev [mailto:core-libs-dev-boun...@openjdk.java.net] On Behalf > Of Joe Wang > Sent: Mittwoch, 17. August 2016 18:30 > To: Aleks Efimov > Cc: core-libs-dev > Subject: Re: RFR (JAXP): 8146961: Fix PermGen memory leaks caused by static > final Exceptions > > Looks good, Aleksej. Thanks for coming up with a well-thought solution > to get rid of the static RE field. > > Best, > Joe > > On 8/17/16, 7:04 AM, Aleks Efimov wrote: > > Hi Joe, Aleksey, > > Thank you for reviewing the initial fix. > > > > I followed the Joe's suggestion (thanks for that) and removed static > > 'abort' field completely, the functionality was replaced by throwing > > the exception of newly added type - AbortException. The new webrev > > with removed 'abort' can be found here: > > http://cr.openjdk.java.net/~aefimov/8146961/9/01 > > > > The Tomcat reproducer attached to the bug report fails without the fix > > and passes with the new version of the fix. > > The JPRT and JCK testing again shows no related jaxp tests failures. > > > > Best Regards, > > Aleksej > > > > > > On 15/08/16 21:09, Joe Wang wrote: > >> Hi Aleksej, > >> > >> I suggest we get rid of the static abort. If RuntimeException > >> happens, check where it happens. The first use case looks suspicious > >> as it just returns if it's an instance of RE. For the 2nd case in DOM > >> error report, let's throw a RuntimeException with the specified error > >> message if error happens, and there's no handler or the handler > >> failed to handle it. (would be better than an empty RE) > >> > >> Best, > >> Joe > >> > >> On 8/15/16, 10:38 AM, Aleks Efimov wrote: > >>> Hi, > >>> > >>> Please, help to review the fix for memory leak [1] in > >>> com.sun.org.apache.xerces.internal.dom.DOMNormalizer that is caused > >>> by usage of static final exceptions. > >>> This problem was already fixed in Apache Xerces project [2] and I > >>> would like to backport it to JDK9 Xerces implementation. > >>> Webrev with the changes: > >>> http://cr.openjdk.java.net/~aefimov/8146961/9/00 > >>> > >>> The Tomcat reproducer attached to the bug report fails without the > >>> fix and passes with the fix. > >>> The JPRT and JCK testing shows no jaxp tests failures with the > >>> proposed fix. > >>> > >>> With Best Regards, > >>> Aleksej > >>> > >>> [1] https://bugs.openjdk.java.net/browse/JDK-8146961 > >>> [2] https://issues.apache.org/jira/browse/XERCESJ-1667 > >>> > >
Re: RFR (JAXP): 8146961: Fix PermGen memory leaks caused by static final Exceptions
Looks good, Aleksej. Thanks for coming up with a well-thought solution to get rid of the static RE field. Best, Joe On 8/17/16, 7:04 AM, Aleks Efimov wrote: Hi Joe, Aleksey, Thank you for reviewing the initial fix. I followed the Joe's suggestion (thanks for that) and removed static 'abort' field completely, the functionality was replaced by throwing the exception of newly added type - AbortException. The new webrev with removed 'abort' can be found here: http://cr.openjdk.java.net/~aefimov/8146961/9/01 The Tomcat reproducer attached to the bug report fails without the fix and passes with the new version of the fix. The JPRT and JCK testing again shows no related jaxp tests failures. Best Regards, Aleksej On 15/08/16 21:09, Joe Wang wrote: Hi Aleksej, I suggest we get rid of the static abort. If RuntimeException happens, check where it happens. The first use case looks suspicious as it just returns if it's an instance of RE. For the 2nd case in DOM error report, let's throw a RuntimeException with the specified error message if error happens, and there's no handler or the handler failed to handle it. (would be better than an empty RE) Best, Joe On 8/15/16, 10:38 AM, Aleks Efimov wrote: Hi, Please, help to review the fix for memory leak [1] in com.sun.org.apache.xerces.internal.dom.DOMNormalizer that is caused by usage of static final exceptions. This problem was already fixed in Apache Xerces project [2] and I would like to backport it to JDK9 Xerces implementation. Webrev with the changes: http://cr.openjdk.java.net/~aefimov/8146961/9/00 The Tomcat reproducer attached to the bug report fails without the fix and passes with the fix. The JPRT and JCK testing shows no jaxp tests failures with the proposed fix. With Best Regards, Aleksej [1] https://bugs.openjdk.java.net/browse/JDK-8146961 [2] https://issues.apache.org/jira/browse/XERCESJ-1667
Re: RFR (JAXP): 8146961: Fix PermGen memory leaks caused by static final Exceptions
Hi Joe, Aleksey, Thank you for reviewing the initial fix. I followed the Joe's suggestion (thanks for that) and removed static 'abort' field completely, the functionality was replaced by throwing the exception of newly added type - AbortException. The new webrev with removed 'abort' can be found here: http://cr.openjdk.java.net/~aefimov/8146961/9/01 The Tomcat reproducer attached to the bug report fails without the fix and passes with the new version of the fix. The JPRT and JCK testing again shows no related jaxp tests failures. Best Regards, Aleksej On 15/08/16 21:09, Joe Wang wrote: Hi Aleksej, I suggest we get rid of the static abort. If RuntimeException happens, check where it happens. The first use case looks suspicious as it just returns if it's an instance of RE. For the 2nd case in DOM error report, let's throw a RuntimeException with the specified error message if error happens, and there's no handler or the handler failed to handle it. (would be better than an empty RE) Best, Joe On 8/15/16, 10:38 AM, Aleks Efimov wrote: Hi, Please, help to review the fix for memory leak [1] in com.sun.org.apache.xerces.internal.dom.DOMNormalizer that is caused by usage of static final exceptions. This problem was already fixed in Apache Xerces project [2] and I would like to backport it to JDK9 Xerces implementation. Webrev with the changes: http://cr.openjdk.java.net/~aefimov/8146961/9/00 The Tomcat reproducer attached to the bug report fails without the fix and passes with the fix. The JPRT and JCK testing shows no jaxp tests failures with the proposed fix. With Best Regards, Aleksej [1] https://bugs.openjdk.java.net/browse/JDK-8146961 [2] https://issues.apache.org/jira/browse/XERCESJ-1667
Re: RFR (JAXP): 8146961: Fix PermGen memory leaks caused by static final Exceptions
Hi Aleksej, Note that DOM spec about error handling: If errors occur during the invocation of this method, such as an attempt to update a read-only node or a |Node.nodeName| contains an invalid character according to the XML version in use, errors or warnings (|DOMError.SEVERITY_ERROR| or |DOMError.SEVERITY_WARNING|) will be reported using the |DOMErrorHandler| object associated with the "error-handler " parameter. Note this method might also report fatal errors ( |DOMError.SEVERITY_FATAL_ERROR|) if an implementation cannot recover from an error. So in this case, it doesn't seem to be "processing aborted by the user" as noted where RE was caught. When a DOM error happens, e.g. not wellformed, if no errorHandler is registered, the process will abort without an error. It's possible to handle the case without throw&catch RE. Please investigate. Thanks, Joe On 8/15/16, 11:09 AM, Joe Wang wrote: Hi Aleksej, I suggest we get rid of the static abort. If RuntimeException happens, check where it happens. The first use case looks suspicious as it just returns if it's an instance of RE. For the 2nd case in DOM error report, let's throw a RuntimeException with the specified error message if error happens, and there's no handler or the handler failed to handle it. (would be better than an empty RE) Best, Joe On 8/15/16, 10:38 AM, Aleks Efimov wrote: Hi, Please, help to review the fix for memory leak [1] in com.sun.org.apache.xerces.internal.dom.DOMNormalizer that is caused by usage of static final exceptions. This problem was already fixed in Apache Xerces project [2] and I would like to backport it to JDK9 Xerces implementation. Webrev with the changes: http://cr.openjdk.java.net/~aefimov/8146961/9/00 The Tomcat reproducer attached to the bug report fails without the fix and passes with the fix. The JPRT and JCK testing shows no jaxp tests failures with the proposed fix. With Best Regards, Aleksej [1] https://bugs.openjdk.java.net/browse/JDK-8146961 [2] https://issues.apache.org/jira/browse/XERCESJ-1667
Re: RFR (JAXP): 8146961: Fix PermGen memory leaks caused by static final Exceptions
Hi Aleksej, I suggest we get rid of the static abort. If RuntimeException happens, check where it happens. The first use case looks suspicious as it just returns if it's an instance of RE. For the 2nd case in DOM error report, let's throw a RuntimeException with the specified error message if error happens, and there's no handler or the handler failed to handle it. (would be better than an empty RE) Best, Joe On 8/15/16, 10:38 AM, Aleks Efimov wrote: Hi, Please, help to review the fix for memory leak [1] in com.sun.org.apache.xerces.internal.dom.DOMNormalizer that is caused by usage of static final exceptions. This problem was already fixed in Apache Xerces project [2] and I would like to backport it to JDK9 Xerces implementation. Webrev with the changes: http://cr.openjdk.java.net/~aefimov/8146961/9/00 The Tomcat reproducer attached to the bug report fails without the fix and passes with the fix. The JPRT and JCK testing shows no jaxp tests failures with the proposed fix. With Best Regards, Aleksej [1] https://bugs.openjdk.java.net/browse/JDK-8146961 [2] https://issues.apache.org/jira/browse/XERCESJ-1667
Re: RFR (JAXP): 8146961: Fix PermGen memory leaks caused by static final Exceptions
On 08/15/2016 08:38 PM, Aleks Efimov wrote: > This problem was already fixed in Apache Xerces project [2] and I > would like to backport it to JDK9 Xerces implementation. Webrev with > the changes: http://cr.openjdk.java.net/~aefimov/8146961/9/00 Ouch. Looks good. Thanks, -Aleksey
RFR (JAXP): 8146961: Fix PermGen memory leaks caused by static final Exceptions
Hi, Please, help to review the fix for memory leak [1] in com.sun.org.apache.xerces.internal.dom.DOMNormalizer that is caused by usage of static final exceptions. This problem was already fixed in Apache Xerces project [2] and I would like to backport it to JDK9 Xerces implementation. Webrev with the changes: http://cr.openjdk.java.net/~aefimov/8146961/9/00 The Tomcat reproducer attached to the bug report fails without the fix and passes with the fix. The JPRT and JCK testing shows no jaxp tests failures with the proposed fix. With Best Regards, Aleksej [1] https://bugs.openjdk.java.net/browse/JDK-8146961 [2] https://issues.apache.org/jira/browse/XERCESJ-1667
