Re: Infinite loop in PrettyWriter

[Per Bothner <per at bothner dot com>]

Wen-Chun Ni wrote:
> When I was testing my program, certain string (FString really) will
> cause the program to hang.
I recently ran into a PrettyWriter hang, but haven't been able to
chase it down.  Your input helped me find one possible culprit.
Could you try the attached patch?  If it doesn't solve the problem,
I also added a bunch of comments that may help you understand
enough of how the queue works to debug it.
--
	--Per Bothner
per@bothner.com   http://www.bothner.com/per/
Index: PrettyWriter.java
===================================================================
RCS file: /cvs/kawa/kawa/gnu/text/PrettyWriter.java,v
retrieving revision 1.5
diff -u -r1.5 PrettyWriter.java
--- PrettyWriter.java	2 Jun 2002 21:18:15 -0000	1.5
+++ PrettyWriter.java	29 Nov 2002 20:31:26 -0000
@@ -104,10 +104,29 @@
    * to output the buffer. The length is stored in the logical block stack. */
   char[] suffix = new char[initialBufferSize];
 
+  // We have a queue of pending operations.  This is primarily stored
+  // in the circular buffer queueInt.  There are different kinds of
+  // operation types, and each operation can require a variable number
+  // of elements in the buffer, depending on the operation type.  Given
+  // an operation at 'index', the type operation type code is
+  // 'getQueueType(index)' (one of the QUEUED_OP_XXX_TYPE macros
+  // below), and the number of elements in the buffer is
+  // 'getQueueSize(index)' (one of the QUEUED_OP_XXX_SIZE values
+  // below).  You can think of the various QUEUED_OP_XXX_TYPEs as
+  // "sub-classes" of queued operations, but instead of creating
+  // actual Java objects, we allocate the objects' fields in the
+  // queueInts and QueueStrings arrays, to avoid expensive object
+  // allocation.  The special QUEUED_OP_DUMMY_TYPE is a used as a
+  // marker for when there isn't enough space in the rest of buffer,
+  // so we have to wrap around to the start.  The other QUEUED_OP_XXX
+  // macros are the offsets of the various "fields" relative to the
+  // start index.
+      
   static final int QUEUE_INIT_ALLOC_SIZE = 300; // FIXME
   int[] queueInts = new int[QUEUE_INIT_ALLOC_SIZE];
   /** For simplicity, queueStrings is the same size as queueInts. */
   String[] queueStrings = new String[QUEUE_INIT_ALLOC_SIZE];
+  /** Index in queueInts and queueStrings of oldest enqueued operation. */
   int queueTail;
   /** Number of elements (in queueInts and queueStrings) in use. */
   int queueSize;
@@ -593,7 +612,7 @@
       return colinc - (column - origin) % colinc;
   }
 
-  int indexColumn(int index) // DONE
+  int indexColumn(int index)
   {
     int column = bufferStartColumn;
     int sectionStart = getSectionColumn();
@@ -602,22 +621,22 @@
     int todo = queueSize;
     while (todo > 0)
       {
-	// If at end of queueInt, or it's a 1-word QUEUED_OP_DUMMY_TYPE, skip.
-	if (op >= queueInts.length - 1)
+	// If at end of queueInt, skip.
+	if (op >= queueInts.length)
 	  op = 0;
-	int posn = queueInts[op + QUEUED_OP_POSN];
-	if (posn >= endPosn)
-	  break;
 	int type = getQueueType(op);
-	if (type == QUEUED_OP_TAB_TYPE)
-	  {
-	    column += computeTabSize(op, sectionStart,
-				     column + posnIndex (posn));
-	  }
-	else if (type == QUEUED_OP_NEWLINE_TYPE
-		 || type == QUEUED_OP_BLOCK_START_TYPE)
+	if (type != QUEUED_OP_DUMMY_TYPE)
 	  {
-	    sectionStart = column + posnIndex(queueInts[op + QUEUED_OP_POSN]);
+	    int posn = queueInts[op + QUEUED_OP_POSN];
+	    if (posn >= endPosn)
+	      break;
+	    if (type == QUEUED_OP_TAB_TYPE)
+	      column += computeTabSize(op, sectionStart,
+				       column + posnIndex (posn));
+	    else if (type == QUEUED_OP_NEWLINE_TYPE
+		     || type == QUEUED_OP_BLOCK_START_TYPE)
+	      sectionStart
+		= column + posnIndex(queueInts[op + QUEUED_OP_POSN]);
 	  }
 	int size = getQueueSize(op);
 	todo -= size;