From bec9c68bf3d101b1533edd7f1fef03f5995c6148 Mon Sep 17 00:00:00 2001
From: DS <ds.desour@proton.me>
Date: Sun, 30 Apr 2023 18:20:48 +0200
Subject: [PATCH] Release invlist resizelock while doing the recursive callback
 in move_somewhere mode (#13470)

Fixes a crash in popular creative inventory mods that set the list when you put
something into trash.
---
 src/inventorymanager.cpp | 58 +++++++++++++++++++++++++---------------
 1 file changed, 36 insertions(+), 22 deletions(-)

diff --git a/src/inventorymanager.cpp b/src/inventorymanager.cpp
index 1c6b3ffc9..e1bb42c48 100644
--- a/src/inventorymanager.cpp
+++ b/src/inventorymanager.cpp
@@ -260,12 +260,18 @@ void IMoveAction::apply(InventoryManager *mgr, ServerActiveObject *player, IGame
 		return;
 	}
 
-	InventoryList *list_from = inv_from->getList(from_list);
-	InventoryList *list_to = inv_to->getList(to_list);
+	auto get_borrow_checked_invlist = [](Inventory *inv, const std::string &listname)
+			-> InventoryList::ResizeLocked
+	{
+		InventoryList *list = inv->getList(listname);
+		if (!list)
+			return nullptr;
+		return list->resizeLock();
+	};
+
+	auto list_from = get_borrow_checked_invlist(inv_from, from_list);
+	auto list_to = get_borrow_checked_invlist(inv_to, to_list);
 
-	/*
-		If a list doesn't exist or the source item doesn't exist
-	*/
 	if (!list_from) {
 		infostream << "IMoveAction::apply(): FAIL: source list not found: "
 			<< "from_inv=\"" << from_inv.dump() << "\""
@@ -279,10 +285,9 @@ void IMoveAction::apply(InventoryManager *mgr, ServerActiveObject *player, IGame
 		return;
 	}
 
-	auto list_from_lock = list_from->resizeLock();
-	auto list_to_lock = list_to->resizeLock();
-
 	if (move_somewhere) {
+		list_from.reset();
+
 		s16 old_to_i = to_i;
 		u16 old_count = count;
 		caused_by_move_somewhere = true;
@@ -300,23 +305,32 @@ void IMoveAction::apply(InventoryManager *mgr, ServerActiveObject *player, IGame
 
 		// Try to add the item to destination list
 		s16 dest_size = list_to->getSize();
+		auto add_to = [&](s16 dest_i) {
+			// release resize lock while the callbacks are happening
+			list_to.reset();
+
+			to_i = dest_i;
+			apply(mgr, player, gamedef);
+			assert(move_count <= count);
+			count -= move_count;
+
+			list_to = get_borrow_checked_invlist(inv_to, to_list);
+			if (!list_to) {
+				// list_to was removed. simulate an empty list
+				dest_size = 0;
+				return;
+			}
+			dest_size = list_to->getSize();
+		};
 		// First try all the non-empty slots
 		for (s16 dest_i = 0; dest_i < dest_size && count > 0; dest_i++) {
-			if (!list_to->getItem(dest_i).empty()) {
-				to_i = dest_i;
-				apply(mgr, player, gamedef);
-				assert(move_count <= count);
-				count -= move_count;
-			}
+			if (!list_to->getItem(dest_i).empty())
+				add_to(dest_i);
 		}
-
 		// Then try all the empty ones
 		for (s16 dest_i = 0; dest_i < dest_size && count > 0; dest_i++) {
-			if (list_to->getItem(dest_i).empty()) {
-				to_i = dest_i;
-				apply(mgr, player, gamedef);
-				count -= move_count;
-			}
+			if (list_to->getItem(dest_i).empty())
+				add_to(dest_i);
 		}
 
 		to_i = old_to_i;
@@ -482,7 +496,7 @@ void IMoveAction::apply(InventoryManager *mgr, ServerActiveObject *player, IGame
 	*/
 	bool did_swap = false;
 	move_count = list_from->moveItem(from_i,
-		list_to, to_i, count, allow_swap, &did_swap);
+		list_to.get(), to_i, count, allow_swap, &did_swap);
 	if (caused_by_move_somewhere)
 		count = old_count;
 	assert(allow_swap == did_swap);
@@ -574,7 +588,7 @@ void IMoveAction::apply(InventoryManager *mgr, ServerActiveObject *player, IGame
 	/*
 		Report move to endpoints
 	*/
-	list_to_lock.reset();
+	list_to.reset();
 
 	// Source = destination => move
 	if (from_inv == to_inv) {