Skip to content

Fix GUI concurrent access and autocrafter deadlock#38

Open
Distolfix wants to merge 4 commits intoSongoda-Plugins:developmentfrom
Distolfix:fix/gui-concurrent-autocrafter-deadlock
Open

Fix GUI concurrent access and autocrafter deadlock#38
Distolfix wants to merge 4 commits intoSongoda-Plugins:developmentfrom
Distolfix:fix/gui-concurrent-autocrafter-deadlock

Conversation

@Distolfix
Copy link
Copy Markdown

@Distolfix Distolfix commented Nov 13, 2025

Summary

This PR fixes three critical issues: GUI concurrent access crashes, autocrafter deadlock, and WildStacker/UltimateStacker item loss.

Problems Fixed

1. GUI Concurrent Access (ConcurrentModificationException)

Multiple players opening the same hopper GUI simultaneously caused server crashes due to unsynchronized iterator access in the GUI update task.

2. Autocrafter Deadlock

When autocrafter was active, it would fill all 5 slots (ingredients + output), preventing any new items from being added. The hopper would become permanently stuck with items unable to enter.

3. WildStacker/UltimateStacker Item Loss

Two separate bugs causing hundreds of items to disappear:

  • Event emission bug: Suction emitted InventoryPickupItemEvent even when hopper was full, causing stacker plugins to modify entity amounts incorrectly
  • Slot reservation bug: Overly conservative slot counting prevented pickup even with 2-3 free slots available

Solutions

GUI Thread Safety

  • Added synchronizedList() wrapper for GUI update task viewer list
  • Ensures thread-safe iteration when multiple players access same hopper
  • Prevents ConcurrentModificationException crashes

Autocrafter Slot Reservation

  • Modified suction and hopper transfer modules to reserve 1 empty slot when autocrafter is active
  • Hopper maintains at least 1 free slot for craft output
  • Uses reserveOneSlot parameter in addAny() to enforce reservation

WildStacker Item Loss Fixes

  • Suction fix: Move addAny() before event emission, only emit event when added > 0
  • Cache fix: Calculate usable slots correctly: (freeSlots - 1) when reserving
  • Partial stacks: Allow filling partial stacks regardless of slot reservation (doesn't consume slots)

Technical Details

Thread Safety Implementation

private final List<UUID> viewers = Collections.synchronizedList(new ArrayList<>());

Slot Reservation Logic

// In suction: reserve slot if autocrafter present
boolean hasAutoCrafter = hopper.getLevel().getModule("AutoCrafting") != null;
int added = hopperCache.addAny(itemStack, toAdd, hasAutoCrafter);

// In cache: track usable slots
int usableFreeSlots = reserveOneSlot ? (freeSlots - 1) : freeSlots;

Event Emission Fix

// Try adding first
int added = hopperCache.addAny(itemStack, toAdd, hasAutoCrafter);

// Only emit event if we actually added items
if (added == 0) {
    continue; // Skip without emitting event
}

// Now safe to emit event
InventoryPickupItemEvent pickupEvent = new InventoryPickupItemEvent(hopperInventory, item);

Testing

  • ✅ Multiple players opening same hopper GUI simultaneously - no crashes
  • ✅ Autocrafter with full inventory - new items can still enter
  • ✅ 10,000 diamonds with suction + autocrafter + WildStacker - no item loss
  • ✅ Items with 2-3 free slots are now properly collected
  • ✅ Partial stacks filled even when reserving slot for autocrafter

Compatibility

  • Compatible with WildStacker, UltimateStacker, and RoseStacker
  • Backwards compatible with vanilla and non-stacker setups
  • No breaking changes to existing functionality

Bug fix Songoda-Plugins#1 - GUI Concurrent Access:
Multiple players can now simultaneously access the same hopper GUI without closing each other's views. Changed activePlayer tracking from single Player to Set<Player>, and implemented proper cleanup when players close their GUIs individually.

Bug fix Songoda-Plugins#2 - Autocrafter Deadlock Prevention:
Fixed issue where hoppers with active autocrafters would fill all slots when thousands of items were on the ground, preventing the autocrafter from functioning. The suction module now reserves slots when an autocrafter is detected, maintaining at least 2 free slots (one for craft output, one for the next craft). Additionally, the autocrafter module will now automatically eject items when completely full of ingredients to prevent deadlocks.
Fix two critical bugs causing item loss with stacker plugins:

1. Suction module item loss prevention
   - Moved addAny() call before event emission
   - Only emit InventoryPickupItemEvent when items are actually added
   - Prevents stacker plugins from modifying entities when hopper is full

2. Storage cache slot reservation fix
   - Fixed overly conservative slot counting for autocrafter
   - Correctly calculates usable slots: (freeSlots - 1) when reserving
   - Allows partial stack filling regardless of reservation
   - Prevents items getting stuck on ground with available slots

Both fixes are essential for servers using WildStacker, UltimateStacker,
or RoseStacker to prevent hundreds of items disappearing during suction
pickup with autocrafter active.
- Fix item duplication with protected items from other plugins
  Add rollbackAdd() method to revert cache changes when InventoryPickupItemEvent is cancelled
  This prevents duplication loop when items are protected by shop plugins

- Fix slot reservation logic for autocrafter
  Only reserve slot when autocrafter has configured recipe, not just when module exists
  Remove overly restrictive slot blocking that prevented partial stack filling

- Improve slot usage calculation
  Correctly handle free slot reservation with (freeSlots - 1) logic
  Allow partial stack filling even when reserving slots for autocrafter
- Save autocrafter configuration to database instead of config file
  Prevents data loss when server crashes or is force-killed

- Use complete ItemStack serialization to preserve custom items
  Support items with custom names, lore, enchantments, and NBT data

- Add automatic migration from old config format to database
  Existing autocrafter configurations are migrated on first load

- Add AUTOCRAFTER type to ItemType enum for database storage
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant