[mbastian]

Problem

ColumnStandardIndexImpl.get(value) took the lock of the column index when it was called and released it only when the returned iterator was exhausted, so the lock was held while the caller's code ran:

• Iterating index.get(column, value) and calling a setter in the loop, while another thread sets the same column or adds a node, deadlocked: the other thread held the table lock or the view index monitor and waited for the index lock, and the setter waited for them. This happens for a view index, for the elements of the view.
• Breaking out of the loop early, or calling get and never iterating, left the lock held for good, so every later setter or addNode on that column blocked.
• Iterating the result twice failed with an IllegalMonitorStateException.

For the columns that are not indexed (ColumnNoIndexImpl), and the degree and edge type columns, get held the graph read lock the same way, and count, values, getMinValue and getMaxValue of a non-indexed column kept the graph read lock when getAttribute threw (a removed column), which blocked every later write to the graph.

Fix

Index.get and ColumnIndex.get return an ElementIterable<T> and follow the convention of the graph iterables: the lock is taken when iterator() is called and held until the iterator is exhausted or doBreak() is called by the iterating thread.

• ColumnStandardIndexImpl.get(value) returns a ValueIterable. Iterators opened on one result are linked, so a single doBreak() releases all of them, and it does nothing after exhaustion or when called twice. toArray, toCollection and toSet copy under the lock and release it before returning. stream() iterates a copy and holds no lock, parallelStream() throws UnsupportedOperationException as in ElementIterableWrapper.
• ColumnNoIndexImpl, DegreeNoIndexImpl and EdgeTypeNoIndexImpl return a FilteredElementIterable, a lazy filter over the graph iterable with the same contract. It releases the graph read lock if the filter or the source throws.
• count, values, getMinValue and getMaxValue of a non-indexed column release the graph read lock when the loop does not finish.
• IndexImpl.get returns an empty iterable typed with the element type for a column the index does not hold.
• The Javadoc of Index.get and ColumnIndex.get states the rule and the limits below.

Compatibility

The return type of Index.get and ColumnIndex.get changes from Iterable to ElementIterable, which extends Iterable. Source code keeps compiling, but this is a binary incompatible change for code compiled against 0.8.10. Nothing in graphstore calls these methods, and Gephi uses values, count, the minimum and the maximum of the indexes but not get(column, value).

Limits (documented)

• While the lock is held, threads that set the indexed column, or write to the graph for a non-indexed column, wait. A loop body that sets attributes, or that calls the graph, model or index APIs, can deadlock with them. Copy the result with toArray() first, it releases the lock before returning.
• An exception thrown by the loop body leaves the lock held unless doBreak() is called.
• A result is iterated and broken from one thread and is not shared between threads.
• A doBreak() releases every open iteration of the same result, so it also ends an outer loop over that result.

Tests

In IndexStoreTest: a setter running while a view index is iterated, an unfinished get, a doBreak after a partial or complete iteration, nested iterations with one doBreak, parallelStream(), stream().findFirst(), the edge type column, toArray() on the empty result of the edge index, and get and the numeric index methods on a non-indexed column after removeColumn, on indexed and non-indexed columns (each fails on master with a timeout, an exception or a wrong result). I also compared get with a model on 200 to 300 random sequences of 500 to 600 operations, with nodes and edges, indexed and non-indexed columns, degree and edge type columns, null, -0.0 and NaN values and two views: no difference.

An indexed ColumnIndex, or a result obtained from it, used after its column was removed now throws the IllegalArgumentException of the non-indexed path where it threw a NullPointerException (count, values, countValues and the entry iterator included), and does not leave the lock held. getMinValue and getMaxValue still throw their UnsupportedOperationException.

🤖 Generated with Claude Code