From cbe454543ebe9ac2babcfbe252e243b632a1d740 Mon Sep 17 00:00:00 2001 From: Marko Lindqvist Date: Sun, 9 Oct 2022 15:14:04 +0300 Subject: [PATCH 26/26] Fix client crash between terrain change and worker unit packets Game state can be inconsistent between receiving the packet changing the tile type, and the packet cancelling worker unit activity. Idle callback happening at that moment caused client to crash. Reported by mortmann See osdn #45781 Signed-off-by: Marko Lindqvist --- common/unit.c | 20 +++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/common/unit.c b/common/unit.c index c91c23f15e..3f5a5e5604 100644 --- a/common/unit.c +++ b/common/unit.c @@ -771,9 +771,23 @@ bool can_unit_do_activity_targeted_at(const struct unit *punit, * change that destroys the target of the other activity */ if (target != NULL && is_build_activity(activity, ptile)) { unit_list_iterate(ptile->units, tunit) { - if (is_build_activity(tunit->activity, ptile) - && !can_extras_coexist(target, tunit->activity_target)) { - return FALSE; + if (is_build_activity(tunit->activity, ptile)) { + /* With bad luck on idle callback timing it's + * possible that the client has already received a tile terrain change + * packet, but not yet the packet removing activity from tunit. + * So, it's possible that tunit was making a transforming Irrigate or Mine + * for the previous terrain, but with the new terrain Irrigate or Mine + * would be considered build activity, and that's what + * above is_build_activity() said. The can_extra_coexist() would + * crash because there is no target defined for the transforming activities. */ + if (tunit->activity_target != NULL) { + if (!can_extras_coexist(target, tunit->activity_target)) { + return FALSE; + } + } else { + fc_assert((tunit->activity == ACTIVITY_IRRIGATE) + || (tunit->activity == ACTIVITY_MINE)); + } } } unit_list_iterate_end; } -- 2.35.1