DO NOT REPLY [Bug 6359] - position() within id() makes xalan hung

[bu...@apache.org]

DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG 
RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT
<http://nagoya.apache.org/bugzilla/show_bug.cgi?id=6359>.
ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND 
INSERTED IN THE BUG DATABASE.

http://nagoya.apache.org/bugzilla/show_bug.cgi?id=6359

position() within id() makes xalan hung





------- Additional Comments From mschilling@edgility.com  2002-03-08 20:06 -------
I have a fix for this in 2.2 (I haven't looked at the changes in this area in
2.3.1).

The problem (in 2.2, anyway) occurs when FuncLast asks for the size of the node
set from the id() function. The stack looks like:

FuncLast.execute()
FuncLast.getCountOfContextNodeList()
FilterExprWalker.getLastPos
NodeSetDTM.getLength()
NodeSetDTM.runTo()


The comment for NodeSetDTM says:

 * Derived classes that implement iterators
 * must override runTo(int index), in order that they may
 * run the iteration to the given index. 

The comment for runTo says

 * @param index Position to advance (or retreat) to, with 
 * 0 requesting the reset ("fresh") position and -1 (or indeed
 * any out-of-bounds value) requesting the final position.

In NodeSetDTM itself runTo sets m_next to the reuested index.  If the reuested
index is -1, it sets m_next to point to the last node in the set (m_firstFree - 1).

What's happening here is that getLength calls runTo(-1), to ensure the iterator
runs to its end. But the iterator has already run through the entire node set,
so its index is *one past* the last node; that is, it's m_firstFree.  Thus
runTo(-1)  *decrements* m_next back (to m_firstFree-1), so that the next call to
this iterator processes the last node again.  Hence the infinite loop: asking
for the length of the iterator causes the iterator to reprocess its last node,
causing FuncLast to execute, which calls getLength, which decrements the
iterator...  

My fix is simple.  It's possibly incorrect, since I don't understand all of 
ideas behind this code, but it seems safe.  The idea behind it is that runTo(-1)
is supposed to ensure that the iterator has run to the end.  It should never
move the iterator backward.  The resulting NodeSetDTM.runTo() is:

      public void runTo(int index)
      {
        if (!m_cacheNodes)
          throw new RuntimeException(
            "This NodeSetDTM can not do indexing or counting functions!");

        if ((index >= 0) && (m_next < m_firstFree))
          m_next = index;
        else if (m_next < m_firstFree - 1) // **** changed ******
          m_next = m_firstFree - 1;
  }

The changed line was previously simply "else".   

It might make more sense to unconditionally have runTo(-1) set m_next to
m_firstFree, since that is really the final position, but I'll leave that
decision to people who understand this bettet than I.