Consistent Assertion Failures with sound enabled when loading maps in a certain order

[Kieran P]

Ubuntu 11.04 (Classic Gnome Desktop - not unity), ATI 5770 with official proprietary drivers

r9430 (and possibly earlier) Loading the maps in any of these orders causes a consistent and immediate assertion failure. Loading the same map twice does not seem to change anything. It does not cause the error when using -quickstart.

acropolis --> celt-iberia --> death_canyon

celt-iberia --> acropolis --> death_canyon

death_canyon --> celt-iberia --> acropolis

death_canyon --> acropolis --> celt-iberia

[edit] Each of these maps has a different default civ. Civ specific music was recently added (and the new tracks are large).

*See Philip's comment below for more information.

---

Assertion failed: "removed" Location: file_cache.cpp:187 (Reserve)

Call stack:

(0x83828d0) /home/brian/0ad/binaries/system/pyrogenesis() [0x83828d0] (0x83333b2) /home/brian/0ad/binaries/system/pyrogenesis() [0x83333b2] (0x83340b0) /home/brian/0ad/binaries/system/pyrogenesis() [0x83340b0] (0x8334814) /home/brian/0ad/binaries/system/pyrogenesis() [0x8334814] (0x8392b89) /home/brian/0ad/binaries/system/pyrogenesis() [0x8392b89] (0x8390bad) /home/brian/0ad/binaries/system/pyrogenesis() [0x8390bad] (0x83481a8) /home/brian/0ad/binaries/system/pyrogenesis() [0x83481a8] (0x835dfbe) /home/brian/0ad/binaries/system/pyrogenesis() [0x835dfbe] (0x8358b63) /home/brian/0ad/binaries/system/pyrogenesis() [0x8358b63] (0x834b1b6) /home/brian/0ad/binaries/system/pyrogenesis() [0x834b1b6] (0x8359bdb) /home/brian/0ad/binaries/system/pyrogenesis() [0x8359bdb] (0x834b1b6) /home/brian/0ad/binaries/system/pyrogenesis() [0x834b1b6] (0x8357ff3) /home/brian/0ad/binaries/system/pyrogenesis() [0x8357ff3] (0x8192ea6) /home/brian/0ad/binaries/system/pyrogenesis() [0x8192ea6] (0xb753026f) /home/brian/0ad/binaries/system/libmozjs-ps-release.so(+0x25626f) [0xb753026f]

errno = 0 (?) OS error = ?

---

[Mark]

Assertion failure. Stack trace is at the bottom. I can't reproduce it if I run './pyrogenesis -quickstart.'

file_cache.cpp(187): Assertion failed: "removed"
Assertion failed: "removed"
Location: file_cache.cpp:187 (Reserve)

Call stack:

(0x82f1902) /0ad/binaries/system/pyrogenesis() [0x82f1902]
(0x82a4f41) /0ad/binaries/system/pyrogenesis() [0x82a4f41]
(0x82a5623) /0ad/binaries/system/pyrogenesis() [0x82a5623]
(0x82a59f4) /0ad/binaries/system/pyrogenesis() [0x82a59f4]
(0x82c1878) /0ad/binaries/system/pyrogenesis() [0x82c1878]
(0x82c0942) /0ad/binaries/system/pyrogenesis() [0x82c0942]
(0x82bfb76) /0ad/binaries/system/pyrogenesis() [0x82bfb76]
(0x8305869) /0ad/binaries/system/pyrogenesis() [0x8305869]
(0x82d1c21) /0ad/binaries/system/pyrogenesis() [0x82d1c21]
(0x82c539e) /0ad/binaries/system/pyrogenesis() [0x82c539e]
(0x82d095c) /0ad/binaries/system/pyrogenesis() [0x82d095c]
(0x82c539e) /0ad/binaries/system/pyrogenesis() [0x82c539e]
(0x82ce348) /0ad/binaries/system/pyrogenesis() [0x82ce348]
(0x814f7f6) /0ad/binaries/system/pyrogenesis() [0x814f7f6]
(0x814f96b) /0ad/binaries/system/pyrogenesis() [0x814f96b]
(0xb7cdd649) /0ad/binaries/system/libmozjs-ps-release.so(+0x236649) [0xb7cdd649]

errno = 0 (?)
OS error = ?

Program received signal SIGTRAP, Trace/breakpoint trap.
0xb7fe1424 in __kernel_vsyscall ()
(gdb) bt
#0  0xb7fe1424 in __kernel_vsyscall ()
#1  0xb75acc36 in kill () from /lib/libc.so.6
#2  0x082f5f8b in debug_break ()
    at ../../../source/lib/sysdep/os/unix/udbg.cpp:43
#3  0x082c1886 in FileCache::Impl::Reserve (this=0x83e5b20, size=8060692)
    at ../../../source/lib/file/vfs/file_cache.cpp:187
#4  0x082c0942 in FileCache::Reserve (this=0x83e70c8, size=8060692)
    at ../../../source/lib/file/vfs/file_cache.cpp:235
#5  0x082bfb76 in VFS::LoadFile(Path const&, boost::shared_ptr<unsigned char>&, unsigned int&) ()
#6  0x08305869 in OpenOggNonstream (vfs=..., pathname=..., stream=...)
    at ../../../source/lib/res/sound/ogg.cpp:277
#7  0x082d1c21 in SndData_reload (sd=0x9a7b584, vfs=..., pathname=..., 
    hsd=2452426600933) at ../../../source/lib/res/sound/snd_mgr.cpp:916
#8  0x082c539e in call_init_and_reload (type=0x83d8ef0, vfs=..., pathname=..., 
    flags=<value optimized out>) at ../../../source/lib/res/h_mgr.cpp:516
#9  alloc_new_handle (type=0x83d8ef0, vfs=..., pathname=..., 
    flags=<value optimized out>) at ../../../source/lib/res/h_mgr.cpp:555
#10 h_alloc (type=0x83d8ef0, vfs=..., pathname=..., 
    flags=<value optimized out>) at ../../../source/lib/res/h_mgr.cpp:588
#11 0x082d095c in snd_data_load (vs=0x9fd475c, vfs=..., pathname=..., 
    hvs=2448131633636) at ../../../source/lib/res/sound/snd_mgr.cpp:989
#12 VSrc_reload (vs=0x9fd475c, vfs=..., pathname=..., hvs=2448131633636)

[Philip Taylor]

Steps to reproduce: Start game normally. Load Death Canyon. Quit. Load Celt-Iberia. Quit. Load Acropolis. Assertion.

I get the same behaviour. Each of those maps is loading a different music track, each of which is currently about 8MB, so the three of them add up to >20MB (the VFS cache size) which looks suspicious.

[Jan Wassenberg]

Confirmed, though not with the above repro steps. The VFS cache size is 32 MB and those tracks add up to 24 MB, so loading the maps worked fine. I can provoke it by having ChooseCacheSize return 16*MiB and inserting the following code before the main loop:

shared_ptr<u8> buf1, buf2, buf3;
size_t size1, size2, size3;
g_VFS->LoadFile(L"audio/music/hellenes_peace_1.ogg", buf1, size1);
g_VFS->LoadFile(L"audio/music/celts_peace_1.ogg", buf2, size2);
g_VFS->LoadFile(L"audio/music/iberians_peace_1.ogg", buf3, size3);

Investigating..

[Jan Wassenberg]

As usual when something blows up, there's a whole lot going wrong here.

1) Streaming is disabled, so these music files are being loaded in one big piece (ouch)

2) The cache is too small to handle "large" files effectively, because it's crippled by the stub implementation of ChooseCacheSize. There's a lot of stuff being evicted and coalescing going on, but this isn't fatal, at least.

3) The deallocation logic is correct and relies on the shared_ptr returned from the cache to go out of scope, thus freeing space in the cache again. In my toy example, this obviously doesn't happen because buf1..3 remain valid. In case of the music, it's not freed due to the SndData caching, which is desirable. The real underlying problems are therefore 1 and to a lesser extent 2.

What I've done is made file_cache and vfs able to tolerate the cache being full of data for which references are still being held. That prevents the crash/error, at least.

[Jan Wassenberg]

(In [9437]) gracefully handle the case where the file cache is full of data still referenced elsewhere. fixes #832

[Kieran P]

@jan: what is the cache size limit at the moment? 32mb? Lol, 128mb for most games! Please increase it!

Re: ChooseCacheSize then please fix it :-) Make it 1/10 of the total ram. (for 4GB of RAM uses 400mb of the RAM for the game!).

[Jan Wassenberg]

Please see #611: Philip has correctly pointed out that the file cache is often redundant, because the data is also usually cached at a higher level. We also don't want to waste precious address space on 32-bit systems (it's all reserved up-front).

There are 2 new thoughts/facts on this issue:

• the improved file code no longer relies on huge BLOCK_SIZE for efficiency, so file sizes are only rounded up to 4K. That gets us much better cache utilization.
• as with Linus' rant on O_DIRECT, there will always need to be a buffer for IO. Allocating a separate buffer would also kick stuff out of the OS cache and fragment the heap, so I see value in providing extra space in the cache beyond the actual size we want to cache for temporary buffers (especially as long as the music files are loaded in their entirety).

I propose a simpler version of ChooseCacheSize based on the following constraints:

0) always provide at least 64 MiB

1) never use more than ~200 MiB on 32-bit systems

2) never use more than total RAM minus 200 MiB for OS minus 300 MiB for the rest of the game

3) never use more than 400 or 500 MiB (total data size)

Does that sound good?

[Kieran P]

Ok, so on a 32 bit 2gb system, it'd use 200mb.

On a 64bit 4 GB system, it'd use 500mb?

That sounds good.

[Jan Wassenberg]

Yep, except for 64-bit systems with less than 1 GB RAM, in which case rule 2 will restrict us to something between 64 and 500.

[Jan Wassenberg]

(In [9445]) fix ChooseCacheSize - more simple, no longer relies on available-memory reporting (because Linux has a weird understanding of "available") closes #611, refs #832