Project

Profile

Help

Bug #6541

closed

Optimization bug: ClassCastException: class ObjectToBeSorted cannot be cast to class GroupToBeSorted

Added by Norm Tovey-Walsh 2 months ago. Updated about 2 months ago.

Status:
Resolved
Priority:
Normal
Assignee:
Category:
Internals
Sprint/Milestone:
-
Start date:
2024-09-19
Due date:
% Done:

0%

Estimated time:
Legacy ID:
Applies to branch:
12, trunk
Fix Committed on Branch:
12, trunk
Fixed in Maintenance Release:
Platforms:
.NET, Java

Description

It took me a little while to work out that this was related to optimization. If you run

java -jar saxon-ee-12.5.jar -s:xml/doclet.xml -xsl:xsl/chunk-xmljavadoc.xsl \
     -o:./result/out base-output=./result

with a license, it crashes:

java.lang.ClassCastException: class [Lnet.sf.saxon.expr.sort.ObjectToBeSorted; cannot be cast to class [Lnet.sf.saxon.expr.sort.GroupToBeSorted; ([Lnet.sf.saxon.expr.sort.ObjectToBeSorted; and [Lnet.sf.saxon.expr.sort.GroupToBeSorted; are in unnamed module of loader 'app')
	at net.sf.saxon.expr.sort.SortedGroupIterator.getSnapShot(SortedGroupIterator.java:91)
	at com.saxonica.config.EnterpriseConfiguration.processResultDocument(EnterpriseConfiguration.java:1924)
	at net.sf.saxon.expr.instruct.ResultDocument$ResultDocumentElaborator.lambda$elaborateForPush$0(ResultDocument.java:958)
	at net.sf.saxon.expr.LetExpression$LetExprElaborator.lambda$elaborateForPush$6(LetExpression.java:942)
	at net.sf.saxon.expr.instruct.ForEachGroup$ForEachGroupElaborator.lambda$elaborateForPush$6(ForEachGroup.java:889)
	at net.sf.saxon.expr.instruct.Block$BlockElaborator.lambda$elaborateForPush$3(Block.java:876)
	at net.sf.saxon.expr.instruct.TemplateRule.applyLeavingTail(TemplateRule.java:376)
	at net.sf.saxon.trans.Mode.handleRuleNotNull(Mode.java:587)
	at net.sf.saxon.trans.Mode.applyTemplates(Mode.java:521)
	at net.sf.saxon.trans.XsltController.applyTemplates(XsltController.java:684)
	at net.sf.saxon.s9api.AbstractXsltTransformer.applyTemplatesToSource(AbstractXsltTransformer.java:430)
	at net.sf.saxon.s9api.Xslt30Transformer.applyTemplates(Xslt30Transformer.java:306)
	at net.sf.saxon.Transform.processFile(Transform.java:1405)
	at net.sf.saxon.Transform.doTransform(Transform.java:894)
...

It works fine without a license, so I conclude it's an EE optimization.


Files

chunk-xmljavadoc.xsl (29.6 KB) chunk-xmljavadoc.xsl Norm Tovey-Walsh, 2024-09-19 17:34
doclet.xml (2.02 MB) doclet.xml Norm Tovey-Walsh, 2024-09-19 17:34
Actions #1

Updated by Michael Kay 2 months ago

Reproduced. Although the problem goes away when you run without a license file, it doesn't go away when you run with -opt:0, and it does go away when you run with --allow-multithreading:off, so I think it's probably something to do with asynchronous handling of xsl:result-document rather than with any rewrite optimisation.

Actions #2

Updated by Michael Kay 2 months ago

This looks like a very basic coding error: in SortedGroupIterator method buildArray, the assignment

values = new ObjectToBeSorted[allocated];

should read

values = new GroupToBeSorted[allocated];

It's one of those bugs that begs the question "How did this ever work?", and "How come the tests didn't catch this?"

Actions #3

Updated by Michael Kay 2 months ago

A query against the test suite shows that there are no tests in which xsl:result-document appears as a descendant of xsl:for-each-group, which is the trigger condition for this bug.

Actions #4

Updated by Norm Tovey-Walsh 2 months ago

The combinatorics of testing every function and every instruction in every (even just every valid) context is a little mind boggling. I wonder if a bunch of them could be generated. I mean, yes, a bunch of them could clearly be generated, but would the low hanging fruit be useful?

Actions #5

Updated by Michael Kay 2 months ago

I refined the search and did find some tests, though most of them are streaming tests, which follow a different path:

xslt40-test/tests/fn/json-to-xml/json-to-xml-B.xsl
xslt40-test/tests/fn/json-to-xml/json-to-xml-B2.xsl 
xslt40-test/tests/strm/si-fork/si-fork-802.xsl 
xslt40-test/tests/strm/si-fork/si-fork-B.xsl 
xslt40-test/tests/strm/si-for-each-group/si-group-032.xsl 
xslt40-test/tests/strm/si-for-each-group/si-group-031.xsl 
xslt40-test/tests/strm/si-for-each-group/si-group-204.xsl 
xslt40-test/tests/strm/si-for-each-group/si-group-205.xsl

and the only two non-streaming stylesheets (json-to-xml-B and -B2) are not actually used in any tests!

Actions #6

Updated by Michael Kay 2 months ago

I resuscitated test json-to-xml-013 which uses the stylesheet json-to-xml-B.xsl, and it didn't hit the bug. Turns out there's another precondition for the bug, which is that the xsl:for-each-group has an xsl:sort child - the problem is in SortedGroupIterator. Confirmed that adding the xsl:sort does trigger the bug, and that the proposed patch fixes it.

Actions #7

Updated by Michael Kay 2 months ago

The combinatorics of testing every function and every instruction in every (even just every valid) context is a little mind boggling.

Indeed. We do make attempts to ensure coverage, for example there is an introspection test to ensure that every combination of XSLT element/attribute permitted by the schema is present in some non-error stylesheet. But there's certainly no attempt to ensure that every permitted parent/child element combination is present. And as the above comment demonstrates, we actually need a combination of three instructions here: xsl:for-each-group/xsl:sort/xsl:result-document.

Another way of approaching this is by looking at code coverage at the Java level. We've attempted that in the past, but the problem is simply that there are too many code fragments that don't get executed to examine them all in detail; and when you do examine them, most of them are there for a good reason, for example if a class has an equals() method then it's right and proper to also have a hashCode() method even though it is never used. I suppose in an ideal world we would also write a unit test to test the hashCode() method - but life is too short.

It appears this bug was introduced between Saxon 10 and 11 - the relevant line of code was correct in Saxon 10, and was changed for some reason.

Actions #8

Updated by Michael Kay 2 months ago

It appears this bug was introduced between Saxon 10 and 11 - the relevant line of code was correct in Saxon 10, and was changed for some reason.

Looking at the commit history, the change was made on 9/3/2021 [I think that's 2021-03-09 - MHK] and was done to enable C# transpilation. I haven't yet checked that the fixed code transpiles successfully. It seems that if a variable is declared with type ObjectToBeSorted[] then in Java we can assign it to an array of type GroupToBeSorted given that GroupToBeSorted is a subtype of ObjectToBeSorted[]; this causes problems in C#. So we may need to do some rethinking.

Actions #9

Updated by Michael Kay about 2 months ago

  • Category set to Internals
  • Status changed from New to Resolved
  • Assignee set to Michael Kay
  • Priority changed from Low to Normal
  • Fix Committed on Branch 12, trunk added
  • Platforms .NET, Java added

Now fixed by creating the array as new ObjectToBeSorted[] rather than new GroupToBeSorted[], and casting the instances to GroupToBeSorted, thus sidestepping the Java/C# difference.

Please register to edit this issue

Also available in: Atom PDF