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.
This commit is contained in:
DS 2023-04-30 18:20:48 +02:00 committed by GitHub
parent b35aa10579
commit bec9c68bf3
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
1 changed files with 36 additions and 22 deletions

View File

@ -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) {