Ticket #43379

Editing a city to have size 255 freezes the server

Open Date: 2021-12-11 11:39 Last Update: 2023-09-30 12:49

Reporter:
(Anonymous)
Owner:
Type:
Status:
Closed
Component:
Priority:
5 - Medium
Severity:
5 - Medium
Resolution:
Fixed
File:
2

Details

To reproduce: - enter edit mode - select a city - set the size to 255 (the maximum allowed) - go do some sport while waiting for the server to complete the growth (killed it after 9 hours on my machine)

Ticket History (3/13 Histories)

2021-12-11 11:39 Updated by: None
  • New Ticket "Editing a city to have size 255 freezes the server" created
2022-07-19 06:47 Updated by: dark-ether
Comment

i tested this and could reproduce in freeciv 3.0, first it sends the message

1: Did not find a cm solution in 27500 iterations for Kraków.

until 206 times, then it does

1: in city_size_add() [city.c::1106]: assertion '0xFF - size > add' failed.

1: Please report this message at https://osdn.net/projects/freeciv/ticket/

and then repeats both messages the cm solution message comes from common/aicore/cm i haven't yet searched for the assertion failure. do note that the city needs to actually reach 255 size to reproduce the bug, so if it doesn't have a sewer system it is not possible to reproduce the bug

(Edited, 2022-07-19 06:51 Updated by: dark-ether)
2022-07-19 07:54 Updated by: dark-ether
Comment

Reply To dark-ether

i tested this and could reproduce in freeciv 3.0, first it sends the message 1: Did not find a cm solution in 27500 iterations for Kraków. until 206 times, then it does 1: in city_size_add() [city.c::1106]: assertion '0xFF - size > add' failed. 1: Please report this message at https://osdn.net/projects/freeciv/ticket/ and then repeats both messages the cm solution message comes from common/aicore/cm i haven't yet searched for the assertion failure. do note that the city needs to actually reach 255 size to reproduce the bug, so if it doesn't have a sewer system it is not possible to reproduce the bug

ok i was browsing the code and if we look at the city_size_change function it calls with a while loop city_increase_size, and besides when it can't grow becuase it grew to the size limit it always returns true this means that if something prevents size from increasing the loop will go on forever, the assertion that fails is a return assertion, so it causes the code to return, i assume this causes size to not increase, i may be wrong and some other thing prevents size from increasing. actually i am blind that is exactly what happens the function that changes size is the city_size_add which gets cancelled when the assert fails

(Edited, 2022-07-19 07:56 Updated by: dark-ether)
2022-07-20 00:23 Updated by: dark-ether
Comment

i have a half fix, i call it half because although it fixes the infinite loop it still takes too long to stop, however to fix it taking too long it would need a substantial change in how it works,

the problem is that it calculates exhaustively the options, which is really slow for large numbers, not only that but it calculates for each value along the way so from around 60 until 254 it makes a lot of useless and time intensive calculations, if it calculated only the last one maybe it would be acceptable.

my suggestion would be to remove things that should happen only once from city_increase_size to a new function city_has_grown which is called only once, however that seems to big for only one ticket.

2023-06-12 10:08 Updated by: cazfi
Comment

Reply To dark-ether

it calculates for each value along the way

That's probably easier part to address currently.

There's also #44703 trying to achieve something similar.

2023-06-12 10:14 Updated by: cazfi
  • Milestone Update from (None) to 3.2.0
  • Component Update from (None) to Server
2023-06-26 20:18 Updated by: cazfi
Comment

Reply To (Anonymous)

- set the size to 255 (the maximum allowed)

Should be allowed, but causes an assert failure -> #48296

2023-09-03 22:39 Updated by: cazfi
  • Owner Update from (None) to cazfi
  • Resolution Update from None to Accepted
2023-09-03 23:12 Updated by: cazfi
Comment

Patch applies, as is, also to S3_1. I want to test it on development branches first, but it's something to likely push to S3_1 after beta3 release.

2023-09-06 03:21 Updated by: cazfi
Comment

Pushed to main and S3_2

2023-09-30 12:49 Updated by: cazfi
  • Status Update from Open to Closed
  • Resolution Update from Accepted to Fixed

Edit

You are not logged in. I you are not logged in, your comment will be treated as an anonymous post. » Login