Aleksei Valikov wrote:
> Hi.
>
> This concerns JAXB 1.0.6.
>
> A Hyperjaxb user send me a bug report about StackOverflowError with his schema.
> I've investigated and it seems to be a problem with XJC.
>
> In his schema (attached) there are several elements with large maxOccurs (like
> maxOccurs="2000").
Right. This is an architectural problem of JAXB 1.0.x.
> The problem is that elements with large maxOccurs produce relatively large trees
> - and when walked, they also produce deep recursion. This causes SOE.
>
> From xjc code it appears that OccurenceExp is often processed as a simple
> expression, where it could be processed as occurence. In many places tree
> walking is simply unnecessary, since the bottom expression (itemExp) is know as
> well as minOccurs and maxOccurs. Therefor considering OccurenceExp in some
> places could remove the unnecessary recursion.
That is probably true. But chances are, even if you clear this one
problem, there will be many more. For example, when we serialize this
expression tree into a file, Java serialization walks the tree in
depth-first fashion, causing SOE. That's why I think this is an
architectural problem. I fixed this in 2.0, so it will never have this
sort of the problem (modulo Xerces which we use unless there's -nv ---
the JAXP team just fixed that, I think.)
> I have identified four of such locations. I'm listing with my corrections:
>
> 1. DefaultParticleBinder.NameCollisionChecker.readSuperClass.
>
> public void onOther(OtherExp exp) {
>
> if(exp instanceof OccurrenceExp) {
> ((OccurrenceExp) exp).itemExp.visit(this);
> return;
> }
> if(exp instanceof FieldItem) {
> occupiedLabels.put(((FieldItem)exp).name,exp);
> return;
> }
> if(exp instanceof IgnoreItem) {
> return;
> }
> exp.exp.visit(this);
> }
>
> 2. AGMFragmentBuilder.findSuperClass.
>
> parent.exp.visit(new BGMWalker() {
> private boolean inSuper;
> private final Set visitedExps = new HashSet(); // inifinite
> recursion control
>
>
>
> public void onOther(OtherExp exp) {
> if (exp instanceof OccurrenceExp)
> {
> ((OccurrenceExp) exp).itemExp.visit(this);
> return;
>
> }
> super.onOther(exp);
> }
> public void onElement(ElementExp exp) {
> if( visitedExps.add(exp) )
> super.onElement(exp);
> }
> public Object onClass(ClassItem item) {
> if(inSuper)
> result[0] = item;
> return null;
> }
>
> public Object onSuper(SuperClassItem item) {
> inSuper = true;
> visitedExps.clear();
> item.exp.visit(this);
> inSuper = false;
> return null;
> }
> });
>
> 3. RelationNormalizer.Pass1.onOther:
> public Expression onOther( OtherExp exp ) {
>
> // if it's not a java item,
> // simply recurse its contents.
> if(!(exp instanceof JavaItem)) {
> // is this OK? looks potentially dangaerous
> // when this OtherExp is shared because
> // the modification can depend on the context.
>
> // but this is necessary to preserve OccurrenceExp
> if (exp instanceof OccurrenceExp)
> {
> OccurrenceExp oexp = (OccurrenceExp) exp;
> Expression p;
> if (oexp.minOccurs == 0)
> {
> p = pool.createZeroOrMore(oexp.itemExp.visit(this));
> }
> else
> {
> p = pool.createOneOrMore(oexp.itemExp.visit(this));
> }
> multiplicity = new Multiplicity(oexp.minOccurs, oexp.maxOccurs);
> return p;
> }
> else
> {
> exp.exp = exp.exp.visit(this);
> return exp;
> }
> }
>
> ...
>
> 4. Obviously somewhere in marshaller/validator generation, an exception appears
> during serialization of the BGM tree. I have not identified exactly where, I've
> simply disabled validation and validation marshaller generation.
>
> I don't want to commit my changes since I'm not 100% sure that they're fine. I'd
> like someone from the JAXB group to recheck them. Thank you!
Go ahead and commit them. It will trigger the automated tests, and if it
wails, I'll back out the changes, and we can discuss those failures.
Your changes look OK to me, so I don't think it causes any problem.
--
Kohsuke Kawaguchi
Sun Microsystems kohsuke.kawaguchi_at_sun.com