Bug #6091
closedNPE in ContextStackIterator.getMajorCaller with Saxon 12
100%
Description
We have some code like this:
protected void attachXSLTStackTrace(TransformerException e, XSLTransformerValidationDPI errorDpi) {
if (e instanceof XPathException) {
XPathException xPathException = (XPathException) e;
XPathContext context = xPathException.getXPathContext();
if (context != null) {
Iterator<ContextStackFrame> iterator = new ContextStackIterator(context);
final StringWriter sw = new StringWriter();
while (iterator.hasNext()) {
ContextStackFrame frame = iterator.next();
frame.print(new net.sf.saxon.lib.Logger() {
@Override
public void println(String message, int severity) {
sw.write(message + "\n");
}
@Override
public StreamResult asStreamResult() {
return new StreamResult(sw);
}
});
}
String entireStackTrace = sw.toString();
if(entireStackTrace.length() > 0){
errorDpi.setDetailedExceptionInfo(new DetailedExceptionInfo("XSLT Stack Trace:\n" + entireStackTrace, null, e.getMessage()));
}
}
}
}
called when we get a falal error from Saxon and now we have a nullpointerexception here with Saxon 12:
java.lang.NullPointerException: null
at net.sf.saxon.trace.ContextStackIterator.getMajorCaller(ContextStackIterator.java:147)
at net.sf.saxon.trace.ContextStackIterator.next(ContextStackIterator.java:79)
at net.sf.saxon.trace.ContextStackIterator.next(ContextStackIterator.java:24)
at ro.sync.xml.transformer.DPIErrorListener.attachXSLTStackTrace(DPIErrorListener.java:438)
at ro.sync.xml.transformer.DPIErrorListener.addError(DPIErrorListener.java:399)
at ro.sync.dxsl.debugger.TrAXErrorListener.addError(TrAXErrorListener.java:78)
at ro.sync.xml.transformer.DPIErrorListener.fatalError(DPIErrorListener.java:358)
at net.sf.saxon.lib.ErrorReporterToListener.report(ErrorReporterToListener.java:68)
at net.sf.saxon.Controller.reportFatalError(Controller.java:444)
at net.sf.saxon.trans.XsltController.applyTemplates(XsltController.java:700)
Can we prohibit this in some way?
Updated by Michael Kay over 1 year ago
The class ContextStackIterator has a comment
// TODO: this class is no longer used by StandardErrorListener.printStackTrace(). It's potentially
// still useful to have a programmatic way of obtaining the stack trace, but this class is rather
// clumsy and probably isn't used.
which almost certainly means there are no unit tests.
But the failing method in Saxon 11 reads:
private static XPathContextMajor getMajorCaller(XPathContext context) {
XPathContext caller = context.getCaller();
while (!(caller == null || caller instanceof XPathContextMajor)) {
caller = caller.getCaller();
}
return (XPathContextMajor) caller;
}
and in Saxon 12 it reads:
private static XPathContextMajor getMajorCaller(XPathContext context) {
XPathContext caller = context.getCaller();
return caller.getMajorContext();
}
The change was made on the 12 branch on 2022-06-24 supposedly "to allow old XPathContextMajor objects to be garbage collected".
It seems unlikely that we would have made this change unless we actually encountered a problem that showed it to be necessary. However, the change to this method was made as part of a larger change that removed duplication in code that navigates the context stack, so I suspect that this was an incorrectly-implemented refactoring to simplify duplicated code, done in the course of solving a performance problem.
I'll change it to:
XPathContext caller = context.getCaller();
return caller == null ? null : caller.getMajorContext();
Updated by Michael Kay over 1 year ago
- Tracker changed from Support to Bug
- Category set to Internals
- Status changed from New to Resolved
- Assignee set to Michael Kay
- Applies to branch 12, trunk added
- Fix Committed on Branch 12, trunk added
- Platforms .NET, Java added
Updated by Radu Coravu over 1 year ago
Thanks Michael! I will attempt the same patch on my side.
Updated by O'Neil Delpratt over 1 year ago
- Status changed from Resolved to Closed
- % Done changed from 0 to 100
- Fixed in Maintenance Release 12.3 added
Bug fix applied in the Saxon 12.3 maintenance release.
Please register to edit this issue