Fix item loss when player disconnects during trade

[GG-MD]

Problem Description

Players were losing items when either trader disconnected during an active trade session. The items would disappear completely - neither player would receive them back. This was a critical bug affecting item safety.

Reproduction Steps

1. Player A and Player B start a trade
2. Both players put items in the trade window
3. One player disconnects (either via network issue or forced disconnect)
4. Both players lose their items - they're gone forever

What I Found

After adding debug logging and testing with two players, I discovered the root cause was a race condition in the abort() method:

1. When a player disconnects, handleQuitTrade() calls trade.abort()
2. abort() was calling end() first, which closed the GUI inventories
3. Closing the GUI triggered handleClose() event handler
4. handleClose() tried to call abort() again (recursion!)
5. But abort() set ended=true early, so the second call would exit before returning items
6. The original abort() call then tried to return items, but the GUI was already closed and cleared
7. Items were lost in this window between GUI closure and item return

Additionally, the old code used ContainerUtils.INSTANCE.addOrDrop() which runs asynchronously. When a player disconnects, they're gone before the async operation completes, so items never get added to their inventory.

Solution

I restructured the abort() method to follow a safer transaction pattern:

Changes in Trade.java

1. Set ended=true immediately at the start of abort() - prevents handleClose() from calling abort() again
2. Collect items synchronously - loop through all trade slots and clone items into lists
3. Clear slots immediately after cloning - prevents item duplication exploits (important!)
4. Return items synchronously using player.getInventory().addItem() instead of async ContainerUtils
5. Drop excess items on the ground if inventory is full and player is online
6. Only check isOnline() before sending messages/sounds to prevent errors
7. Close GUI last - only after items are safely back in inventories

Changes in TradeGui.java

Added if (trade.isEnded()) return; check in handleClose() to prevent recursive abort calls.

Testing

Tested multiple scenarios with two test players:

• Player disconnect during trade: ✅ Items returned correctly
• Both players with full inventories: ✅ Items dropped on ground
• Multiple items (tested with 7 diamond tools): ✅ All items returned
• No item duplication exploits: ✅ Slots cleared immediately after cloning

The fix has been running on my server for several test sessions without any item loss or duplication.

Technical Details

Before:

abort() -> end() -> closeInventory() -> triggers handleClose() -> abort() again
                                          (but ended=true so exits early)
         -> tries to return items but GUI already closed -> items lost

After:

abort() -> ended=true (prevents recursion)
        -> collect & clone items from GUI
        -> clear slots (prevent dupe)
        -> return items synchronously
        -> close GUI safely

This ensures items are always returned before any GUI cleanup happens, and the synchronous approach guarantees items are added even if the player disconnects immediately after.

Additional Notes

I noticed the original code also lacked null checks and online status checks in some places. I added those where appropriate to prevent additional edge case errors.

Let me know if you need any clarification or have questions about the implementation!

[GG-MD]

@BenceX100

[GG-MD]

@BenceX100