[Freeciv-Dev] Re: GTK client crash (assertion failed: too much luxury) (PR#1409)

[Alex Wilkins <awilkins@xxxxxxxxxxxx>]

On Sun, 28 Apr 2002, Raimar Falke wrote:

>On Thu, Apr 25, 2002 at 07:54:12PM -0700, awilkins@xxxxxxxxxxxx wrote:
>> Full_Name: Alexander Wilkins
>> Version: April 23 CVS
>> Distribution: Built from source
>> Client: Gtk+
>> OS: Redhat Linux 7.2 (kernel 2.4.9-31)
>> Submission from: (NULL) (128.12.194.114)
>
>Alexander: can you please try the attached patch?
>
>Mike: I have also added some docu to agents.c and the cma.
>
>       Raimar

When I try it, it again crashes when I try to open a city with too much 
luxury.  It appears that this assertion fails:

  assert(luxury <= cache2.allocated_luxury);

(At the time of the crash, luxury was 252 and cache2.allocated_luxury 
was 183.)  Some rudimentary analysis with gdb seems to indicate that the 
estimate on luxury in ensure_invalid_cache2 is too low.  The 
problem is that if all of a city's trade is used for luxury, the 
total amount of luxury scales as total_trade * 2.5.  

One suggestion would be to use as a bound

    (pcity->tax_bonus * total_trade / 100) * pct_luxury + 
        (pcity->size - 1) * pcity->tax_bonus / 50

where pct_luxury is the percentage of the nation's trade allocated for 
luxury.  

However, since pcity->tax_bonus doesn't appear to be correct in 
update_cache2, set_city_tax_bonus from city.c (or an equivalent 
function) would have to be called first.

I'm not sure if pct_luxury is actually accessible from inside 
update_cache2; if not, it can probably be estimated from the relative 
sizes of pcity->trade_prod and pcity->luxury_total.  Now that I think 
of it, a better bound might be 

    pcity->luxury_total * total_trade / pcity->trade_prod + 
        pcity->size * 5

which would use only the information that is already available.  
Testing this latter bound, it seems to work O.K., although it is a 
little loose (by 105 at most, though).  Since it seems that 
ensure_invalid_cache2 is called every time a worker is changed to an 
elvis, you might be able to get away with 

    pcity->luxury_total * total_trade / pcity->trade_prod +
        pcity->ppl_elvis * 5

instead.

At the minimum, though, ensure_invalid_cache2 should probably check to 
make sure that cache2.allocated_luxury > pcity->total_luxury.

The backtrace is:

#0  0x40370ae1 in __kill () from /lib/i686/libc.so.6
#1  0x403708ba in raise (sig=6) at ../sysdeps/posix/raise.c:27
#2  0x40372062 in abort () at ../sysdeps/generic/abort.c:88
#3  0x4036a200 in __assert_fail () at assert.c:74
#4  0x080c0a8d in get_city_status (luxury=252, workers=0) at cma_core.c:692
#5  0x080c0c35 in update_cache2 (pcity=0x83c7f50, result=0xbfffd6a0)
    at cma_core.c:755
#6  0x080c0e90 in real_fill_out_result (pcity=0x83c7f50, result=0xbfffd6a0)
    at cma_core.c:825
#7  0x080c12e0 in fill_out_result (pcity=0x83c7f50, result=0xbfffd6a0, 
    base_combination=0x8130ea4, scientists=0, taxmen=0) at cma_core.c:1109
#8  0x080c1cfa in optimize_final (pcity=0x83c7f50, parameter=0xbfffd750, 
    best_result=0xbfffd780) at cma_core.c:1556
#9  0x080c2128 in cma_query_result (pcity=0x83c7f50, parameter=0xbfffd750, 
    result=0xbfffd780) at cma_core.c:1795
#10 0x0807f9fc in refresh_cma_dialog (pcity=0x83c7f50, refresh=REFRESH_ALL)
    at cma_fe.c:335
#11 0x08073e5d in refresh_city_dialog (pcity=0x83c7f50) at citydlg.c:413
#12 0x080770ac in create_city_dialog (pcity=0x83c7f50, make_modal=0)
    at citydlg.c:1580
#13 0x08073f7c in popup_city_dialog (pcity=0x83c7f50, make_modal=0)
    at citydlg.c:458
#14 0x08064f44 in do_map_click (xtile=141, ytile=29) at control.c:1130
#15 0x08090da4 in butt_down_mapcanvas (w=0x82b1390, ev=0x82d39b8)
    at mapctrl.c:294
#16 0x400e4aec in gtk_marshal_BOOL__POINTER () from /usr/lib/libgtk-1.2.so.0
#17 0x40118436 in gtk_handlers_run () from /usr/lib/libgtk-1.2.so.0
#18 0x4011776d in gtk_signal_real_emit () from /usr/lib/libgtk-1.2.so.0
#19 0x40115525 in gtk_signal_emit () from /usr/lib/libgtk-1.2.so.0
#20 0x4014fb89 in gtk_widget_event () from /usr/lib/libgtk-1.2.so.0
#21 0x400e4a45 in gtk_propagate_event () from /usr/lib/libgtk-1.2.so.0
#22 0x400e3a6f in gtk_main_do_event () from /usr/lib/libgtk-1.2.so.0
#23 0x40194d7f in gdk_event_dispatch () from /usr/lib/libgdk-1.2.so.0
#24 0x401c7773 in g_main_dispatch () from /usr/lib/libglib-1.2.so.0
#25 0x401c7d39 in g_main_iterate () from /usr/lib/libglib-1.2.so.0
#26 0x401c7eec in g_main_run () from /usr/lib/libglib-1.2.so.0
#27 0x400e3333 in gtk_main () from /usr/lib/libgtk-1.2.so.0
#28 0x0808b9a5 in ui_main (argc=1, argv=0xbfffded4) at gui_main.c:865
#29 0x080601f1 in main (argc=1, argv=0xbfffded4) at civclient.c:207
#30 0x4035e627 in __libc_start_main (main=0x805fcb4 <main>, argc=1, 
    ubp_av=0xbfffded4, init=0x805cc84 <_init>, fini=0x80c2b40 <_fini>, 
    rtld_fini=0x4000dcc4 <_dl_fini>, stack_end=0xbfffdecc)
    at ../sysdeps/generic/libc-start.c:129

Alexander Wilkins