No longer works on Android

[inactive123]

@jdgleaver It now reports 'Failed to load content' when trying to load the shareware version on Android.

It has previously always worked on Android before. This is a regression.

[jdgleaver]

Could you please confirm that it was working before the last commit - i.e. that this not related to the recent buildbot/Android makefile changes? That would be very helpful for debugging purposes!

Either way, I will investigate this as best I can. I can't see how my changes would harm anything, especially since the Linux and Windows versions are fine, but I'll check every avenue...

[andres-asm]

It's working for me with the retail game content.

[inactive123]

I'm not exactly sure when it happened, and I cannot say for sure that it was anybody's commits, so I don't want to single anybody out here.

What I do know, is that the shareware files on the content downloader work on Windows and Linux. I'd appreciate it if we could find out what the problem is together and figure out a solution somehow on Android. I do believe this worked before.

[jdgleaver]

@twinaphex - I wasn't complaining about being singled out ;)

I want this core to be the best it can be - if I made a mistake, I'll gladly hold my hands up. It's just the more information there is, the easier it is to debug.

I made a comment about the issue in the pull request thread. I realise now I should have copied it here for clarity. Here it is:

• Under both Linux and Windows, the updated core loads all content correctly.

• Under Android, only the full version of the base game + the 2 official expansions work correctly. Attempting to run any unofficial expansion or mod causes the base game to load. It is as though the internal '-game' command line option is being ignored completely. And as stated, the 'content downloader' shareware version fails to load at all.

thatman84 - do you happen to know if the last version of the Android core was able to load 'unofficial' content correctly? I know this used to work, but the previous version of the core I was using was from before the buildbot switched to clang (and before the Android jni makefiles were updated) - i.e. I don't know if this content loading problem was an existing issue caused by other factors...

...and thatman84 kindly offered to check if he had the previous version of the core for testing.

I'll try to get some logs and have a poke around tomorrow...

[jdgleaver]

Okay, I need some help here...

I've discovered the nature of the bug and I've found a fix, but I'm not going to make a pull request until I understand why this bug is happening and why the 'fix' does anything at all...

There is some very strange memory 'corruption' going on here. Take a look at the following code from the retro_load_game() function:

tyrquake/common/libretro.c

Lines 920 to 956 in fdf566f

	parms.argv = argv;
	COM_InitArgv(parms.argc, parms.argv);
	parms.argc = com_argc;
	parms.argv = com_argv;
	heap = (unsigned char*)malloc(parms.memsize);
	parms.membase = heap;
	#ifdef NQ_HACK
	if (log_cb)
	log_cb(RETRO_LOG_INFO, "Quake Libretro -- TyrQuake Version %s\n", stringify(TYR_VERSION));
	#endif
	#ifdef QW_HACK
	if (log_cb)
	log_cb(RETRO_LOG_INFO, "QuakeWorld Libretro -- TyrQuake Version %s\n", stringify(TYR_VERSION));
	#endif
	Sys_Init();
	if (!Host_Init(&parms))
	{
	struct retro_message msg;
	char msg_local[256];
	Host_Shutdown();
	snprintf(msg_local, sizeof(msg_local),
	"PAK archive loading failed...");
	msg.msg = msg_local;
	msg.frames = 360;
	if (environ_cb)
	environ_cb(RETRO_ENVIRONMENT_SET_MESSAGE, (void*)&msg);
	return false;
	}

When loading 'unofficial' content, parms.argv[2] contains the game directory name. When running the Android version:

• At line 939, parms.argv[2] is correctly set to the game directory name

• At line 941 (i.e. after calling Sys_Init()), parms.argv[2] is set to '�' (i.e. garbage)

Sys_Init() does nothing other than call 'environ_cb(RETRO_ENVIRONMENT_SET_PIXEL_FORMAT, &fmt)'.

If I move the Sys_Init() call from retro_load_game() to retro_init() then parms.argv remains uncorrupted and all content loads correctly.

Please, can anyone shed any light on this?

[inactive123]

Yeah, that fixes it. Pushing that.

[inactive123]

It does seem to crash now when exiting the core -

the only thing I see in the backtrace is this which is unfortunately not much -

Thread 1 received signal SIGSEGV, Segmentation fault.
0x00007ffad205e9ca in vswprintf () from C:\Windows\System32\msvcrt.dll
(gdb) bt
#0 0x00007ffad205e9ca in vswprintf () from C:\Windows\System32\msvcrt.dll
#1 0x00007ffad20569b9 in msvcrt!fprintf ()
from C:\Windows\System32\msvcrt.dll
#2 0x00000000134899ca in ?? ()
from C:\Games\RetroArch\cores\tyrquake_libretro.dll
#3 0x0000000013484e57 in ?? ()
from C:\Games\RetroArch\cores\tyrquake_libretro.dll
#4 0x0000000013485ee5 in ?? ()
from C:\Games\RetroArch\cores\tyrquake_libretro.dll
#5 0x00000000004027ba in core_unload ()
#6 0x000000000040bbf1 in command_event ()
#7 0x000000000040585c in rarch_ctl ()
#8 0x0000000000418950 in content_load ()
#9 0x000000000041a740 in task_push_start_dummy_core ()
#10 0x000000000040be4e in command_event ()
#11 0x0000000000515367 in action_ok_close_content ()
#12 0x0000000000512ba3 in menu_entry_action ()
#13 0x000000000054b22d in generic_menu_iterate ()
#14 0x00000000004fc2ac in menu_driver_iterate ()
#15 0x00000000004039d4 in runloop_check_state.constprop ()
#16 0x000000000040793f in runloop_iterate ()
#17 0x0000000000401680 in rarch_main ()
#18 0x0000000000812508 in main_getcmdline ()
#19 0x00000000004013f7 in __tmainCRTStartup ()
at C:/repo/mingw-w64-crt-git/src/mingw-w64/mingw-w64-crt/crt/crtexe.c:334
#20 0x00000000004014fb in WinMainCRTStartup ()
at C:/repo/mingw-w64-crt-git/src/mingw-w64/mingw-w64-crt/crt/crtexe.c:184
(gdb) q
A debugging session is active.

[jdgleaver]

It's crashing because you're calling Sys_Quit() after freeing the heap... i.e. you only needed to move Sys_Init()... 🙂

Do you want to fix this, or shall I make a pull request?

[inactive123]

Go ahead and make the PR.

The reason why I moved Sys_Quit() is because I did notice suspicious behavior and I thought that would fix it, but I didn't know Sys_Quit() was contingent on that heap still being allocated.

I think it's good form to have the equivalent function of Sys_Init that is ran in retro_init() also be deinited by its retro_uninit counterpart.

[jdgleaver]

I wasn't criticising (hence the smiley!) - it's completely unobvious that Sys_Quit() requires the heap (it goes down pretty deep in the code).

I agree completely that retro_deinit() should mirror retro_init(), so I've left Sys_Quit() where it is and moved the 'free(heap)' - I'll make the pull request right now.