AttributesImpl.java includes a "fix" that changed the original: public void removeAttribute (int index) { if (index >= 0 && index < length) { if (index < length - 1) { System.arraycopy(data, (index+1)*5, data, index*5, (length-index)*5); } length--; } else { badIndex(index); } } to: public void removeAttribute (int index) { if (index >= 0 && index < length) { data[index*5] = null; data[index*5+1] = null; data[index*5+2] = null; data[index*5+3] = null; data[index*5+4] = null; if (index < length - 1) { System.arraycopy(data, (index+1)*5, data, index*5, (length-index-1)*5); } length--; } else { badIndex(index); } } The original code was clearing the entry at the new end of the list by copying the presumably empty entry "one past the end" over the last entry. The change correctly changed the length of the copy to no longer reference past the end of the data array, which would sometimes raise an ArrayIndexOutOfBoundsException. However, the code to clear the element being removed just before it is copied over can only be useful in the single case where the copy doesn't happen, i.e. the attr removed is the last one in the list. The correct code would always clear the entry that actually needs to be cleared, which is the one that was previously in the last position before one was removed. The following code does so: public void removeAttribute (int index) { if (index >= 0 && index < length) { if (index < length - 1) { System.arraycopy(data, (index+1)*5, data, index*5, (length-index-1)*5); } length--; data[length*5] = null; data[length*5+1] = null; data[length*5+2] = null; data[length*5+3] = null; data[length*5+4] = null; } else { badIndex(index); } }
Do we have a test program that shows the effects of this bug on another Apache product yet?
I am not sure how to create one exactly. The reason for the initial report is not know to me, except that from inspection I can see that the original code did not clear the "slots" following the new last entry after a call to remove an attribute. This would simply leave references to String objects that would not be collectable during gc(), but would otherwise seem to be benign (they are immutable after all...) My changes were to simply take the "spirit" of the original fix and change the code to clear the String references so that the entries in the array were always null beyond the last in-use entry. Perhaps we should ask garyp or dims if they have more information to offer.