[10.5, 10.6 bug] double free error

[isidoro]

Easy.  Keep the interface but change the implementation to STL.  I have not checked if that is easy/feasible or not, though...

[Spike]

I have my (very personal and most likely very hard to explain to C++ programmers) doubts about the STL. It's not that the STL would be bad, but the templates can skyrocket compile times easily (my experience is very old though, maybe current compilers and STL implementations don't have that problem anymore).

It's not about the interfaces. Also not about functionality, bloat or execution speed. STL is alright in regard to these aspects, although I do not like the notation of the idioms. But that is really something personal.

I don't know if it's good to write this now and here. But I want to remove the dependencies to STL which have been established, and strip down the existing templates to the smallest needed set of methods in my branch. I know that a lot of people want more STL in Simutrans and kick out the non-STL templates. Quite the opposite of my ideas and plans.

In this regard it is good that I have my own branch to work on, so I do not bother anyone else with my strange opinions on how to use templates. And if Standard goes full STL it's alright, since STL is _the_ C++ standard for templates and should be used for anything but personal toy projects (says the software professional in me).

The only real reason against STL is that one of the platforms which Prissi usually compiles on has no complete STL implementation, and this platform would have to be dropped if we go full STL. At least this is what I understood from older messages, and IIRC this was the killer argument against STL so far.

[jamespetts]

Hans,

thank you for your input. Do you (or anyone else) know what those incompatible platforms are?

[ras52]

... And I'm now seeing an intermittent segfault coming out of grund_t::neuen_weg_bauen.  (I've checked that I don't have any of my patches for diagonal maintenance applied, so that cannot be the problem.)  Will keep looking.

I'm pretty certain that this is a real heap corruption issue.  My guess is that we're holding on to a pointer after it has been deleted, and are then modifying the deleted object which corrupts malloc's internal data structures.  I have just fixed one such error (in livery_scheme.h) on my Experimental 10.x branch.  The two patches to vector_tpl (1 & 2) should be uncontroversial and could usefully be applied to Standard too.  This hasn't fixed the segfault, so there's presumably another such memory issue yet to be found.  I suspect that the desync bug is probably not related to this, though at the moment this bug is hampering my ability to make any progress with the desync bug.

Edit: An easy way of getting this to fail reproducibly (at least on my machine) is to remove the settings*.xml file, and start simutrans with the pakset on the command line, e.g. ./simutrans -objects Pak128.Britain-Ex-0.8.3.  I reliably get a segfault on load, shortly after selecting 'English' as my language.  Backtrace below.  This error happens with 10.x builds from several weeks ago, so it isn't caused by the recent patches for memory management in e.g. vector_tpl.

Code Select Expand


Show banner ...

Program received signal SIGSEGV, Segmentation fault.
get_hintergrund (season=<optimized out>, hoehe=1, phase=<optimized out>, this=0xd9e8958) at dings/../bauer/../besch/haus_besch.h:74
74            return bild != NULL ? bild->get_nummer() : IMG_LEER;
(gdb) p bild
$1 = (const bild_besch_t * const) 0xad94
(gdb) bt
#0  get_hintergrund (season=<optimized out>, hoehe=1, phase=<optimized out>, this=0xd9e8958) at dings/../bauer/../besch/haus_besch.h:74
#1  gebaeude_t::get_bild (this=0xd97a850, nr=1) at dings/gebaeude.cc:463
#2  0x081b5d7c in ding_t::display (this=0xd97a850, xpos=<optimized out>, ypos=448) at simdings.cc:254
#3  0x0808aed0 in local_display_dinge_bg (reset_dirty=<optimized out>, ypos=<optimized out>, xpos=576, ding=<optimized out>)
    at dataobj/dingliste.cc:1095
#4  dingliste_t::display_dinge_bg (this=0xdb4dee8, xpos=576, ypos=576, start_offset=1 '\001', reset_dirty=true) at dataobj/dingliste.cc:1115
#5  0x080802ba in grund_t::display_dinge_bg (this=0xdb4dee4, xpos=576, ypos=576, is_global=true, draw_ways=false, visible=true)
    at boden/grund.cc:1163
#6  0x08081beb in grund_t::display_dinge_all (this=0xdb386f4, xpos=640, ypos=544, raster_tile_width=128, is_global=true) at boden/grund.cc:1096
#7  0x081dfc1a in planquadrat_t::display_dinge (this=0xdac257c, xpos=640, ypos=544, raster_tile_width=128, is_global=true, hmin=2 '\002',
    hmax=127 '\177') at simplan.cc:386
#8  0x081e27de in karte_ansicht_t::display (this=0xd921180, force_dirty=true) at simview.cc:199
#9  0x081cdd2c in intr_refresh_display (dirty=false) at simintr.cc:75
#10 0x0820e38b in karte_t::sync_step (this=0xda8e978, delta_t=33, sync=true, display=true) at simworld.cc:3037
#11 0x081d1c13 in modal_dialogue (gui=0xdf43ea8, magic=-1, welt=0xda8e978, quit=0x81d1960 <never_quit()>) at simmain.cc:223
#12 0x081d4b66 in simu_main (argc=3, argv=0x7fffffff) at simmain.cc:1122
#13 0x0824a8b9 in main (argc=3, argv=0xbffff2c4) at simsys_s.cc:682


[sdog]

"utils/for.h" from standard's svn is missing in the experimental 10.x branches but required by simsys

[jamespetts]

Oops - forgot to add this. Now added.

[ras52]

Apologies.  That looks like it was my fault.  I'm still getting used to driving git.

[Markohs]

 Just my 5 cents.

Not that I'm a C++ expert (I've allways been more in PHP,javascript and shell-scripting), but refering to what Hans/Hajo said, I don't thiink compile times should be a factor to decide against using STL.

If it can avoid us future bugs it should be welcome imo, even at the expense of compile times. Can't see how compilation times should skyroket anyways, given any build system only recompiles the minimum set of source files needed. So it will really matters if you are modyfing .h files included by many files, and that should not be *that* common. I'm I wrong?

And by the way, I'd like to know how much of our players really use Haiku/AmigaOS, because I don't really think they should stop us improving the code in any way. And I fail to see how any modern PDA/videoconsole/phone won't support a modern C++ compiler. (if it really has one and it's not java like android).

EDIT: Just to state it clear, I don't really know it STL whould improve simutrans by some factor, even my intuition says it prolly will. I'm just saying that compilation times shouldn't not be a factor to decide against them. But that's just my oppinion. I refrained myself from using a STL queue in sumutrans3d and implemented it with a simutrans slist iirc.

[isidoro]

Just my 5 cents.
[...]

I think that you are 3 cents ahead... 

[prissi]

Soem short remarks:

Haiku uses GCC 2.96 and has incomplete STL. The GCC does not support SDL completely.

MVS 6.0 could not even compile:

Code Select Expand


for( int i=0;  i<10; i++ ) {
...
}

for( int i=0;  i<10; i++ ) {
...
}

MSVC throwed an error in the second for loop for double declared i ... You will find still some traces from making Simutrans compatible with MSVC 6.0 from 2004, most in makeobj.

The minivec was originally part of the dinglist, and since contained in every tile on the map it had to be small. Later dingliste came up with a direct management, same a planquadrat. As such 4 byte per vector (which mostly had one or none element) was quite significant.

STL for most structures, if it makes sense, is probably the future.

(The verctor_tpl copying orperators I made private, since they are not used in standard.)

(Personally I really dislike functions empty() to test on a emty object. My conotation is to empty a list without deleting objects, and clear to clear the list i.e. also delete the objects). But I am maybe just too old or too conservative.)

[Dwachs]

Imho, we are a good way into fixing the double-free errors. Of course, there is a transition phase now, where some bugs and compile errors pop up. Personally, I do not see any need for using STL containers (just a feeling, no rational arguments).

[Markohs]

I think that you are 3 cents ahead... 

hahaha.

[Ashley]

Code Select Expand

Feb 19 17:33:56 pulsar sim64[9839]: ERROR: network_receive_data:#011connection [10] already closed
Feb 19 17:33:56 pulsar sim64[9839]: Please report all errors to
Feb 19 17:33:56 pulsar sim64[9839]: team@64.simutrans.com
Feb 19 17:34:38 pulsar sim64[9839]: Warning: nwc_tool_t::rdwr:#011rdwr id=6 client=0 plnr=14 pos=465,476,-1 wkzid=8223 defpar=p14,Thanwer Trans init=1 flags=0
Feb 19 17:34:38 pulsar sim64[9839]: Warning: network_check_activity():#011received cmd id=6 nwc_tool_t from socket[8]
Feb 19 17:34:38 pulsar sim64[9839]: Warning: nwc_tool_t::clone:#011send sync_steps=16437  wkz=8223 init
Feb 19 17:34:38 pulsar sim64[9839]: Warning: nwc_tool_t::rdwr:#011rdwr id=6 client=4 plnr=14 pos=465,476,-1 wkzid=8223 defpar=p14,Thanwer Trans init=1 flags=0
Feb 19 17:34:42 pulsar sim64[9839]: Warning: network_check_activity():#011received cmd id=10 nwc_auth_player_t from socket[8]
Feb 19 17:34:42 pulsar sim64[9839]: Warning: network_check_activity():#011received cmd id=10 nwc_auth_player_t from socket[8]
Feb 19 17:36:01 pulsar sim64[9839]: Warning: nwc_tool_t::rdwr:#011rdwr id=6 client=0 plnr=14 pos=482,491,-1 wkzid=4110 defpar=gavel_road init=0 flags=0
Feb 19 17:36:01 pulsar sim64[9839]: Warning: network_check_activity():#011received cmd id=6 nwc_tool_t from socket[8]
Feb 19 17:36:01 pulsar sim64[9839]: Warning: nwc_tool_t::clone:#011send sync_steps=16952  wkz=4110 work
Feb 19 17:36:01 pulsar sim64[9839]: Warning: nwc_tool_t::rdwr:#011rdwr id=6 client=4 plnr=14 pos=482,491,-1 wkzid=4110 defpar=gavel_road init=0 flags=0
Feb 19 17:36:01 pulsar sim64[9839]: Warning: two_click_werkzeug_t::init:#011
Feb 19 17:36:01 pulsar sim64[9839]: Warning: two_click_werkzeug_t::init:#011
Feb 19 17:36:01 pulsar sim64[9839]: *** glibc detected *** /var/testsim/instances/sim64/sim: double free or corruption (out): 0x09a8fa50 ***
Feb 19 17:36:01 pulsar sim64[9839]: ======= Backtrace: =========
Feb 19 17:36:01 pulsar sim64[9839]: /lib/i686/nosegneg/libc.so.6(+0x6c0d1)[0xb75760d1]
Feb 19 17:36:01 pulsar sim64[9839]: /lib/i686/nosegneg/libc.so.6(+0x6d928)[0xb7577928]
Feb 19 17:36:01 pulsar sim64[9839]: /lib/i686/nosegneg/libc.so.6(cfree+0x6d)[0xb757aa4d]
Feb 19 17:36:01 pulsar sim64[9839]: /usr/lib/libstdc++.so.6(_ZdlPv+0x21)[0xb7754701]
Feb 19 17:36:01 pulsar sim64[9839]: /usr/lib/libstdc++.so.6(_ZdaPv+0x1d)[0xb775475d]
Feb 19 17:36:01 pulsar sim64[9839]: /var/testsim/instances/sim64/sim[0x80a991d]
Feb 19 17:36:01 pulsar sim64[9839]: /var/testsim/instances/sim64/sim[0x80aa050]
Feb 19 17:36:01 pulsar sim64[9839]: /var/testsim/instances/sim64/sim[0x828ef8f]
Feb 19 17:36:01 pulsar sim64[9839]: /var/testsim/instances/sim64/sim[0x823789b]
Feb 19 17:36:01 pulsar sim64[9839]: /var/testsim/instances/sim64/sim[0x824658c]
Feb 19 17:36:01 pulsar sim64[9839]: /var/testsim/instances/sim64/sim[0x82b2a7f]
Feb 19 17:36:01 pulsar sim64[9839]: /lib/i686/nosegneg/libc.so.6(__libc_start_main+0xe6)[0xb7520ca6]
Feb 19 17:36:01 pulsar sim64[9839]: /var/testsim/instances/sim64/sim[0x804a7e1]
Feb 19 17:36:01 pulsar sim64[9839]: ======= Memory map: ========
Feb 19 17:36:01 pulsar sim64[9839]: 08048000-08312000 r-xp 00000000 ca:01 215616     /var/testsim/instances/sim64/sim
Feb 19 17:36:01 pulsar sim64[9839]: 08312000-08315000 rw-p 002c9000 ca:01 215616     /var/testsim/instances/sim64/sim
Feb 19 17:36:01 pulsar sim64[9839]: 08315000-083ab000 rw-p 00000000 00:00 0
Feb 19 17:36:01 pulsar sim64[9839]: 088a0000-09afb000 rw-p 00000000 00:00 0          [heap]
Feb 19 17:36:01 pulsar sim64[9839]: b5d00000-b5d21000 rw-p 00000000 00:00 0
Feb 19 17:36:01 pulsar sim64[9839]: b5d21000-b5e00000 ---p 00000000 00:00 0
Feb 19 17:36:01 pulsar sim64[9839]: b5ec9000-b74dd000 rw-p 00000000 00:00 0
Feb 19 17:36:01 pulsar sim64[9839]: b74dd000-b74ed000 r-xp 00000000 ca:01 901171     /lib/i686/nosegneg/libresolv-2.11.3.so
Feb 19 17:36:01 pulsar sim64[9839]: b74ed000-b74ee000 r--p 00010000 ca:01 901171     /lib/i686/nosegneg/libresolv-2.11.3.so
Feb 19 17:36:01 pulsar sim64[9839]: b74ee000-b74ef000 rw-p 00011000 ca:01 901171     /lib/i686/nosegneg/libresolv-2.11.3.so
Feb 19 17:36:01 pulsar sim64[9839]: b74ef000-b74f1000 rw-p 00000000 00:00 0
Feb 19 17:36:01 pulsar sim64[9839]: b74f1000-b74f5000 r-xp 00000000 ca:01 901186     /lib/i686/nosegneg/libnss_dns-2.11.3.so
Feb 19 17:36:01 pulsar sim64[9839]: b74f5000-b74f6000 r--p 00004000 ca:01 901186     /lib/i686/nosegneg/libnss_dns-2.11.3.so
Feb 19 17:36:01 pulsar sim64[9839]: b74f6000-b74f7000 rw-p 00005000 ca:01 901186     /lib/i686/nosegneg/libnss_dns-2.11.3.so
Feb 19 17:36:01 pulsar sim64[9839]: b74f7000-b7501000 r-xp 00000000 ca:01 901172     /lib/i686/nosegneg/libnss_files-2.11.3.so
Feb 19 17:36:01 pulsar sim64[9839]: b7501000-b7502000 r--p 00009000 ca:01 901172     /lib/i686/nosegneg/libnss_files-2.11.3.so
Feb 19 17:36:01 pulsar sim64[9839]: b7502000-b7503000 rw-p 0000a000 ca:01 901172     /lib/i686/nosegneg/libnss_files-2.11.3.so
Feb 19 17:36:01 pulsar sim64[9839]: b7508000-b750a000 rw-p 00000000 00:00 0
Feb 19 17:36:01 pulsar sim64[9839]: b750a000-b764e000 r-xp 00000000 ca:01 901185     /lib/i686/nosegneg/libc-2.11.3.so
Feb 19 17:36:01 pulsar sim64[9839]: b764e000-b7650000 r--p 00144000 ca:01 901185     /lib/i686/nosegneg/libc-2.11.3.so
Feb 19 17:36:01 pulsar sim64[9839]: b7650000-b7651000 rw-p 00146000 ca:01 901185     /lib/i686/nosegneg/libc-2.11.3.so
Feb 19 17:36:01 pulsar sim64[9839]: b7651000-b7654000 rw-p 00000000 00:00 0
Feb 19 17:36:01 pulsar sim64[9839]: b7654000-b7671000 r-xp 00000000 ca:01 418037     /lib/libgcc_s.so.1
Feb 19 17:36:01 pulsar sim64[9839]: b7671000-b7672000 rw-p 0001c000 ca:01 418037     /lib/libgcc_s.so.1
Feb 19 17:36:01 pulsar sim64[9839]: b7672000-b7696000 r-xp 00000000 ca:01 901167     /lib/i686/nosegneg/libm-2.11.3.so
Feb 19 17:36:01 pulsar sim64[9839]: b7696000-b7697000 r--p 00023000 ca:01 901167     /lib/i686/nosegneg/libm-2.11.3.so
Feb 19 17:36:01 pulsar sim64[9839]: b7697000-b7698000 rw-p 00024000 ca:01 901167     /lib/i686/nosegneg/libm-2.11.3.so
Feb 19 17:36:01 pulsar sim64[9839]: b7698000-b7699000 rw-p 00000000 00:00 0
Feb 19 17:36:01 pulsar sim64[9839]: b7699000-b7782000 r-xp 00000000 ca:01 541295     /usr/lib/libstdc++.so.6.0.13
Feb 19 17:36:01 pulsar sim64[9839]: b7782000-b7786000 r--p 000e9000 ca:01 541295     /usr/lib/libstdc++.so.6.0.13
Feb 19 17:36:01 pulsar sim64[9839]: b7786000-b7787000 rw-p 000ed000 ca:01 541295     /usr/lib/libstdc++.so.6.0.13
Feb 19 17:36:01 pulsar sim64[9839]: b7787000-b778e000 rw-p 00000000 00:00 0
Feb 19 17:36:01 pulsar sim64[9839]: b778e000-b779e000 r-xp 00000000 ca:01 417894     /lib/libbz2.so.1.0.4
Feb 19 17:36:01 pulsar sim64[9839]: b779e000-b779f000 rw-p 00010000 ca:01 417894     /lib/libbz2.so.1.0.4
Feb 19 17:36:01 pulsar sim64[9839]: b779f000-b77b2000 r-xp 00000000 ca:01 540811     /usr/lib/libz.so.1.2.3.4
Feb 19 17:36:01 pulsar sim64[9839]: b77b2000-b77b3000 rw-p 00013000 ca:01 540811     /usr/lib/libz.so.1.2.3.4
Feb 19 17:36:01 pulsar sim64[9839]: b77b7000-b77bc000 rw-p 00000000 00:00 0
Feb 19 17:36:01 pulsar sim64[9839]: b77bc000-b77bd000 r-xp 00000000 00:00 0          [vdso]
Feb 19 17:36:01 pulsar sim64[9839]: b77bd000-b77d8000 r-xp 00000000 ca:01 701013     /lib/ld-2.11.3.so
Feb 19 17:36:01 pulsar sim64[9839]: b77d8000-b77d9000 r--p 0001b000 ca:01 701013     /lib/ld-2.11.3.so
Feb 19 17:36:01 pulsar sim64[9839]: b77d9000-b77da000 rw-p 0001c000 ca:01 701013     /lib/ld-2.11.3.so
Feb 19 17:36:01 pulsar sim64[9839]: bfe2f000-bfe44000 rw-p 00000000 00:00 0          [stack]
Feb 19 17:36:01 pulsar sim64[9839]: Calculating textures ...done


Still seeing this with revision 5382.

[Dwachs]

Please retry with r5389. I could reproduce such a crash now (and add a fix...)

[Ashley]

The new stable has definitely reduced the amount of crashing, only time will tell of course

[Dwachs]

There are still crashes ?

[jamespetts]

The new stable has definitely reduced the amount of crashing, only time will tell of course

Excellent! Hopefully, this effect will propagate back to Experimental, wherein the problem was originally reported, when I eventually finish converting all of the iterators to the new types and merging in all the updates up to 111.2.1.

[Ashley]

There are still crashes ?

Not so far!