[mbastian]

Problem

A setter racing removeColumn failed in three ways:

• Element.setAttribute checked the column and then wrote the value into the slot of the column without a lock, so a column removed in between made the write fail with an ArrayIndexOutOfBoundsException (the store id of the column became -1), or the index update fail with a NullPointerException (IndexImpl.set found no index for the column).
• The setter could write its value into the slot after removeColumn had cleared it. A column added later in the same slot then read a value of the wrong type, and addColumn failed with a ClassCastException.
• The same applied to the variants with a timestamp or an interval, setAttribute(column, value, time) and removeAttribute(column, time).

With a stress test of 10,000 remove and add rounds against one setter thread, master threw 333 NullPointerExceptions, 253 ArrayIndexOutOfBoundsExceptions and 247 ClassCastExceptions.

Fix

No lock is added to the setters (a table lock in the setters deadlocks with iterating an index):

• AttributesImpl writes the value only if the column is still registered in the store (columns[index] == column), checked and written in one hold of the attributes monitor of the element. This covers setAttribute, removeAttribute and their time variants. Otherwise the setter throws the IllegalArgumentException that a call on an already removed column throws.
• ColumnStore.removeColumn removes the column from the store before it clears the values. A value is therefore either written while the column is registered and then cleared, or the setter sees the column is gone.
• removeColumn checks that the column passed in is the registered one before it changes anything. Removing a stale handle of a column removed earlier used to unregister the column that had been added with the same key.
• IndexImpl.set skips the update when the column has no index, which happens while a column is added or after it is removed.
• ColumnStore.removeColumn(String) looks the key up under the graph and table locks, so a column removed and added again in between is not removed instead of the one named.
• AttributesImpl.removeTimeAttribute no longer fails with an ArrayIndexOutOfBoundsException when the attributes of the element are shorter than the index of the column.

Tests

• IndexStoreTest.testSetAttributeWhileRemovingColumn and testSetTimeAttributeWhileRemovingColumn: a setter thread against 20,000 rounds of removeColumn and addColumn that reuse the slot with another type. Only the IllegalArgumentException may leave the setter, and the value in the slot has the type of the last column. Both fail on master.
• IndexStoreTest.testRemoveTimeAttributeOfColumnAddedAfterNodeCreated: removing a time attribute of an element whose attributes are shorter than the column index returns null instead of throwing.
• ColumnStoreTest.testRemoveColumnTwiceKeepsColumnWithSameKey: removing a stale handle fails with an IllegalArgumentException and leaves the column added later with the same key working.

Setting an attribute costs the same as before within noise (about 330 to 350 ns for an indexed Integer column in the store).

Not covered

• An element that is outside the store when a column is removed keeps the old value in that slot, and a column added later in the slot reads it. This is also the case on master and needs a different design.
• The time index update that follows a time write is not guarded, as before. A value written just before removeColumn can have its times released before the setter adds them to the time index.
• clearAttributes() can throw the IllegalArgumentException if a column is removed while it runs.
• A separate race between addNode and time setters (TimestampMap.toKeysArray) still exists.

🤖 Generated with Claude Code