Fix races on the pending item transaction lists

[tricrotism]

Summary

Item transactions (pickup, drop, throw, shoot, craft, trade, anvil, item break) are staged in per-player lists that listener threads write and the consumer thread reads and clears. The two sides are not coordinated, so items can be logged twice or not at all. This change makes the append atomic, makes the consumer take each list whole, and keeps the prepared copy so a retried relational batch still writes its rows.

The problem

Each listener stages its item in a ConcurrentHashMap<String, List<ItemStack>> in ConfigHandler keyed by user.x.y.z, then queues an ITEM_TRANSACTION with a generation id from Queue.getItemId. For example EntityPickupItemListener.java:29-33:

int itemId = getItemId(loggingItemId);
List<ItemStack> list = ConfigHandler.itemsPickup.getOrDefault(loggingItemId, new ArrayList<>());
list.add(itemStack.clone());
ConfigHandler.itemsPickup.put(loggingItemId, list);

The same pattern is in CraftItemListener.java:42-63, PlayerDropItemListener.java:44-48, PlayerItemBreakListener.java:26-30 and ProjectileLaunchListener.java:39-55. These run on the main thread on Paper and on region threads on Folia.

On the consumer thread, ItemTransactionProcess.process (ItemTransactionProcess.java:34-45) calls ItemLogger.prepare, which copies each list with source.get(key) and toArray (ItemLogger.java:73-76), writes the rows, and then calls clearItemTransaction, which removes all nine lists and the generation counter.

The map is concurrent but the lists inside it are plain ArrayLists shared across threads, and the get, add and put are three separate steps. Three things go wrong:

1. Lost rows. A player picks up an item while the consumer is between the copy and clearItemTransaction. The item is added to the list that is about to be removed. clearItemTransaction removes it and the counter, and the transaction queued for that pickup finds no counter and writes nothing.
2. Duplicate rows. A listener calls getOrDefault and gets the current list, the consumer copies and removes it, then the listener adds and puts the old list back. The next transaction logs the items that were already logged plus the new one.
3. Torn reads. ArrayList.add running during toArray on another thread can throw or copy a partly written array.

The anvil path is worse. InventoryChangeListener.java:553-561 does itemsDestroy.put(key, newList) and itemsCreate.put(key, newList), replacing any items already pending for that player and block, for example a craft done a moment earlier at the same spot.

The fix

• utility/ItemUtils.java: new addPendingItems(map, key, items...), which appends inside ConcurrentHashMap.compute. The append and the consumer's remove lock the same bin, so an item goes either into the list being taken or into a new list.
• EntityPickupItemListener, CraftItemListener, PlayerDropItemListener, PlayerItemBreakListener, ProjectileLaunchListener, and the anvil path in InventoryChangeListener: use addPendingItems instead of get, add and put (the anvil path now appends instead of replacing). getItemId is called after the append so the generation that gets queued is at least as new as the item it covers.
• database/logger/ItemLogger.java: snapshot becomes take and uses remove instead of get. Once a list is removed, no listener can reach it, so reading it is safe.
• consumer/process/ItemTransactionProcess.java: the lists are already gone after prepare, so instead of clearItemTransaction only the counter is dropped, with loggingItem.remove(id, generation). If a listener bumped the counter in the meantime, the remove does nothing and the newer queued transaction logs the newer items. The early return for "all lists empty" drops the counter the same way, otherwise it would stay in the map. clearItemTransaction is still used by discard.

The prepared copy is now stored in consumerObjects for every database type. Upstream only stores it for columnar databases. For MySQL and SQLite, if a batch is rolled back before commit and retained for retry, the retry calls process again with the original Location. Upstream had already cleared the lists and counter at that point, so the retry wrote nothing and the rows were lost. With a destructive take the same thing would happen, so the relational path now logs from the stored PreparedTransaction on retry, the same way columnar does. PreparedTransaction.log still skips blacklisted users, so the old early return in ItemLogger.log is not needed on this path. ItemLogger.log has no remaining callers but is left in place.

Rejected: synchronizing each list, or swapping in CopyOnWriteArrayList. Either fixes the torn read but not the lost and duplicate rows, which come from the get, add, put sequence racing the consumer's remove.

Behaviour change

• None for players.
• An anvil use no longer discards other items pending for the same player and block.
• A relational consumer batch that is retried after a failed commit now writes its item transaction rows.

Risk

• Generation handling. I checked both orderings of a listener append against the consumer's take and counter remove. In each case every item ends up in exactly one logged list. getItemId stays synchronized on Queue.class as upstream has it. loggingItem is a Collections.synchronizedMap, whose remove(key, value) is atomic.
• Storing PreparedTransaction for relational databases means Process.discardFailedConsumerData no longer sees a Location for that id and skips ItemTransactionProcess.discard. That is fine because the lists and counter are already gone by then. Columnar databases already behave this way.
• compute holds the bin lock only for a list append, so the hot listener paths do not get slower in any way that matters.

Testing

Build: mvn package passes.
Row parity: a 47-step scenario plugin (blocks, containers, hoppers, pistons, fluids, dispensers, bone meal, explosions, mob deaths, item drops and pickups, then rollback and restore) on Paper 26.2 and Folia 1.21.11 with SQLite, compared order-insensitively against upstream 3af1079 run the same way. The only differing rows on either platform are the plant that bone meal happens to grow (random on every run). No new errors. The race itself needs a burst of item events racing a consumer pass, which the scenario does not produce, so this run shows no regression rather than the fix.

Suggested live test: on MySQL or SQLite, have a player repeatedly drop and pick up the same stack in one spot while a second client crafts and uses an anvil at that block, for about a minute with the consumer busy. Then /co lookup action:+item,-item user:<player> radius:2 should show every drop, pickup, craft and anvil input and output exactly once. Repeat on Folia. For the retry path, stop the database for a few seconds while items are being dropped and check that the drops show up after it comes back.

[netlify]

❌ Deploy Preview for coreprotect failed. Why did it fail? →

Name	Link
🔨 Latest commit	110d0e3
🔍 Latest deploy log	https://app.netlify.com/projects/coreprotect/deploys/6ab3edfd09d27900081a6f56

[Intelli]

Thanks -- automated review is requesting the following changes:

1. Coordinate the pending-item append and generation registration with ItemTransactionProcess.discard(). Currently, a listener can append an item and then block in getItemId() while discard clears that new item along with an older failed transaction. The subsequently queued transaction has nothing left to log.
2. Preserve detached lists if preparation throws. take() removes a list before cloning it, but the prepared transaction is only stored after all preparation succeeds. A cloning exception therefore loses the removed groups before a retry can recover them.

Please preserve the atomic append/take behavior while closing these failure paths.