Strange freezes when punching self in the face in ROOM.WAD #1076
Comments
|
Doesn't the PWAD have an embedded DEHACKED lump? |
No, the dehacked patch is a separate file. ROOM is from 2000, it was a common practice back then. |
|
Unable to reproduce on Linux. Does this happen on Chocolate Doom? You typoed the command line, missing the dot in what is supposed to be "room.wad" Running under Valgrind with memory pooling disabled gives the following: This happens during load time. Pretty clearly a real positive, the context is accessed after freeing. Also code is identical in Chocolate doom, someone (else 😄) should go and fix that. But this is not our culprit for this bug. Not sure what this is. Now these look interesting. There's some kind of issue when the player dies. In normal mode with memory pooling enabled this would not crash or show up in a normal debugger. I don't see how this code could result in an infinite loop but this same issue could hit some other code and cause all kinds of funky bugs. |
|
Also is the issue more likely on certain levels? I only tried |
|
@turol Thank for investigating this! Interestingly, MBF does not remove mobj thinkers immediately [1], but sets their function pointer to a delayed removal function [2]. Maybe I should adapt that same approach for Crispy. But, anyway, it would be nice to have a reproducible test case for that. [1] https://github.com/fabiangreffrath/woof/blob/c86904de68642c81c2cc7b61482fe200bf0b481e/src/p_tick.c#L169 |
I suppose that the maps MAP05-MAP07 with multiple monsters are the most likely ones where the freeze will happen, but I encountered the freeze in MAP03 with a single monster as well, not sure if that zombieman must be alerted to reproduce this or not. Yes, the freeze happens in Chocolate Doom, but I suggest using So Doom for testing as there you can recover health and armor with cheat codes and punch yourself infinitely until the freeze happens.
Fixed that, thanks! |
|
If this happens in Chocolate then it should be reported (and hopefully fixed) there. But then it's also likely that it would also happen in vanilla. Ran a bunch of test in Chocolate and also got these: The first one looks like something that could result in infinite loop but in this case it didn't. These did not happen reliably. What we need now is a reproducible test case, preferably a demo whose playback freezes. |
|
The second one is suspicious: We are reading data in |
|
I have nothing technical to add, but I did notice this in the wad's txt file: Known Bugs: currently you can't exit MAP08... rare crashing with the fist, impossible to exit levels on the baby skill |
That's why I reported this in Crispy tracker, just in case it's present in vanilla and is to be preserved in Choco according to philosophy.
The WAD author knew about this, couldn't fix but at least let us know. |
|
I set |
|
If you can reliably reproduce this can you get a backtrace from the hang? Can we figure out a way to flush the demo file from this state? |
|
I will try to get some output from GDB if the debug build from Ninja freezes. |
|
Ninja debug build won't show anything in GDB upon freeze. How can I get the demo file out? It isn't saved after game freezes. also make sure to zero the direct damage from arch-vile attack in p_enemy.c,
|
|
The freeze occurs in |
|
You should be able to force writing the demo by doing this in GDB: This also terminates the game, just like if you pressed Press Ctrl+C in gdb window to breakpoint if the game has frozen. Or we could add some way to sync write the demo but that would require some surgery of that code. Can you get the backtrace? I really want to see where exactly the infinite loop is happening. |
When the game freezes under GDB (I managed to freeze te game in Choco this time), I cannot either type or paste anything into the MSYS2 terminal where GDB is running :( Other than that, it looks like that in }`
|
|
Does Ctrl+C work while the game is still running normally? It works on Linux... You might have to install either WSL (not sure if it supports graphics though) or a Linux VM. @fabiangreffrath Any thoughts on how to make the game flush the demo synchronously? |
On my Win10 system even when the game is running normally, MSYS2 console doesn't respond to any typing or pasting. |
Thank you, I'll try tomorrow :) |
|
Got it: Thread 1 "crispy-doom" received signal SIGINT, Interrupt.
0x00005555555abbf6 in PIT_CheckThing (thing=0x7ffff4c0ac40) at p_map.c:296
296 if (!(thing->flags & (MF_SOLID|MF_SPECIAL|MF_SHOOTABLE) ))
(gdb) bt
#0 0x00005555555abbf6 in PIT_CheckThing (thing=0x7ffff4c0ac40) at p_map.c:296
#1 0x00005555555adeee in P_BlockThingsIterator
(x=x@entry=1, y=y@entry=8, func=func@entry=0x5555555abbf0 <PIT_CheckThing>)
at p_maputl.c:529
#2 0x00005555555acab4 in P_CheckPosition
(thing=thing@entry=0x7ffff4be1cb8, x=x@entry=-3757650, y=y@entry=-35771695)
at p_map.c:540
#3 0x00005555555acb8f in P_TryMove
(thing=thing@entry=0x7ffff4be1cb8, x=-3757650, y=-35771695) at p_map.c:576
#4 0x00005555555aee6f in P_XYMovement (mo=mo@entry=0x7ffff4be1cb8)
at p_mobj.c:214
#5 0x00005555555af388 in P_XYMovement (mo=0x7ffff4be1cb8) at p_mobj.c:170
#6 P_MobjThinker (mobj=0x7ffff4be1cb8) at p_mobj.c:585
#7 0x00005555555bd54b in P_RunThinkers () at p_tick.c:113
#8 0x00005555555bd5d3 in P_Ticker () at p_tick.c:151
#9 0x000055555559e415 in G_Ticker () at g_game.c:1372
#10 0x000055555557601d in TryRunTics () at d_loop.c:816
#11 0x0000555555594632 in D_RunFrame () at d_main.c:540
#12 0x0000555555594787 in D_DoomLoop () at d_main.c:610
#13 0x000055555559610d in D_DoomMain () at d_main.c:2405
#14 0x000055555557201f in main (argc=<optimized out>, argv=0x7fffffffdf18)
at i_main.c:87
(gdb) n
P_BlockThingsIterator (x=x@entry=1, y=y@entry=8, func=func@entry=0x5555555abbf0 <PIT_CheckThing>)
at p_maputl.c:527
527 mobj = mobj->bnext)
(gdb) n
526 mobj ;
(gdb) n
529 if (!func( mobj ) )
(gdb) n
527 mobj = mobj->bnext)
(gdb) n
526 mobj ;
(gdb) n
529 if (!func( mobj ) )
(gdb) n
527 mobj = mobj->bnext)
(gdb) n
526 mobj ;
(gdb) n
529 if (!func( mobj ) )
(gdb) n
527 mobj = mobj->bnext) |
|
Did you dump the demo from that? I whipped this up, can you give it a try: diff --git a/src/doom/p_maputl.c b/src/doom/p_maputl.c
index 332fc5544..70ffafe40 100644
--- a/src/doom/p_maputl.c
+++ b/src/doom/p_maputl.c
@@ -512,6 +512,7 @@ P_BlockThingsIterator
boolean(*func)(mobj_t*) )
{
mobj_t* mobj;
+ mobj_t *fast = NULL;
if ( x<0
|| y<0
@@ -522,12 +523,34 @@ P_BlockThingsIterator
}
- for (mobj = blocklinks[y*bmapwidth+x] ;
- mobj ;
- mobj = mobj->bnext)
+ mobj = blocklinks[y*bmapwidth+x] ;
+ if (mobj) {
+ fast = mobj->bnext;
+ }
+ for (;
+ ;
+ )
{
- if (!func( mobj ) )
- return false;
+ if (!mobj) {
+ break;
+ }
+
+ if (mobj == fast) {
+ printf("error infinite loop");
+ abort();
+ }
+
+ if (!func( mobj ) )
+ return false;
+
+ mobj = mobj->bnext;
+ if (fast) {
+ fast = fast->bnext;
+ if (fast) {
+ fast = fast->bnext;
+ }
+
+ }
}
return true;
}
Still doesn't seem to hang/abort for me reliably. |
@turol , what does this code do? Is it meant to fix the freezes printing to the console every time a freeze is prevented? Sorry for the closing/reopening, I accidentally clicked the button on the phone screen. |
|
It aborts if the loop is infinite instead of hanging. There's a better version merged now. Enable Also someone please dump the demo if you can reproduce this. |
Is
Does it work the way that if debug is enabled, then the game aborts and the demo file is saved? |
Yes
No, it just aborts. There's a call command somewhere above which can be used to dump the demo from a debugger. Or you could edit the code yourself to call that function before aborting. |
|
I have recorded a freezing demo. To play it back properly, check out this branch. Here's the demo, ROOM.WAD, ROOM.DEH and the batch file containing the command line to play the demo back: |
|
It reproduces. Using timedemo is faster. With Valgrind and memory pooling disabled: https://gist.github.com/turol/a4c48d3866b13bb6fb447958b909a6a6 Lots of problems. Not sure which one it is or why but definitely use after free. Could be modify-while-iterating bug. Also still kinda slow even with timedemo under Valgrind. A shorter demo would still be useful. You could add more calls of that debug macro to places mentioned in those traces. This should help catch it as close to the cause as possible. |
I managed to record a much shorter demo The location and orientation where I catch the infinite loop most easily, just in case, may have to do with the room corner and the ceiling lamp sector border Feel free to add the loop debugging code everywhere you deem necessary in this branch (where I added the demo dumping line to it), |
|
Does this fix it? (Sorry, can't test myself right now) --- a/src/doom/p_tick.c
+++ b/src/doom/p_tick.c
@@ -113,6 +113,10 @@ void P_RunThinkers (void)
currentthinker->function.acp1 (currentthinker);
nextthinker = currentthinker->next;
}
+
+ if (nextthinker == currentthinker)
+ break;
+
currentthinker = nextthinker;
}
|
Unfortunately not, the game still freezes. |
|
I now suspect that the culprit for the loop lies in https://github.com/fabiangreffrath/crispy-doom/blob/master/src/doom/p_maputl.c#L344 |
|
Great to hear that you have an idea! I'll gladly test any patches that you come up with. |
|
This seems to do the trick for me: --- a/src/doom/p_maputl.c
+++ b/src/doom/p_maputl.c
@@ -513,6 +513,7 @@ P_BlockThingsIterator
boolean(*func)(mobj_t*) )
{
mobj_t* mobj;
+ mobj_t* first;
if ( x<0
|| y<0
@@ -524,12 +525,14 @@ P_BlockThingsIterator
LINKED_LIST_CHECK_NO_CYCLE(mobj_t, blocklinks[y*bmapwidth+x], bnext);
- for (mobj = blocklinks[y*bmapwidth+x] ;
+ for (mobj = blocklinks[y*bmapwidth+x], first = mobj ;
mobj ;
mobj = mobj->bnext)
{
if (!func( mobj ) )
return false;
+ if (mobj->bnext == first)
+ break;
}
return true;
} |
|
Nope, desyncs. But the idea is there... |
|
Seems to be working better in this variant: diff --git a/src/doom/p_maputl.c b/src/doom/p_maputl.c
index ffccb8f5..83db1ac4 100644
--- a/src/doom/p_maputl.c
+++ b/src/doom/p_maputl.c
@@ -513,6 +513,7 @@ P_BlockThingsIterator
boolean(*func)(mobj_t*) )
{
mobj_t* mobj;
+ mobj_t* first;
if ( x<0
|| y<0
@@ -524,10 +525,12 @@ P_BlockThingsIterator
LINKED_LIST_CHECK_NO_CYCLE(mobj_t, blocklinks[y*bmapwidth+x], bnext);
- for (mobj = blocklinks[y*bmapwidth+x] ;
+ for (mobj = blocklinks[y*bmapwidth+x], first = mobj ;
mobj ;
mobj = mobj->bnext)
{
+ if (mobj->bnext == first)
+ break;
if (!func( mobj ) )
return false;
} |
|
I don't think |
If I understand correctly, in this case we don't attempt to continue playing in the situation that occurs in my demo in ROOM, right? Is it possible to continue playing in the situation that leads to freeze in Vanilla/Choco? |
|
I think that's one of the cases where the solution will differ a bit for Chocolate and Crispy. Other ports, e.g. MBF, won't hang here as well. |
Yes
Not in a way that's vanilla compatible. |
Should we stick to Vanilla compatibility in this specific case? I and people who appreciate this kind if humor would love to be able to play ROOM, recording demos or not, in Crispy without freezes. Currently ROOM can cause a freeze even not recording a demo. |
|
What we mean is: Chocolate will probably get a "fix" that will detect the infinite loop and error out (Vanilla would loop forever). Crispy will get a slightly different fix that will break the loop and continue. No compatibility lost. Actually, there are thousands of Vanilla complevel demos out there recorded with e.g. PrBoom+ that would cause Vanilla to crash, or hang or do whatever strange stuff that simply does not trigger in a limit-revoming port. |
Are there? I think triggering this requires some very strange DeHackery and there are very few wads which do that. |
|
I did not mean crash or hang by this specific bug, but there are dozens of ways to kill the Vanilla engine that ports starting with Boom circumvent while remaining demo compatible. This specific issue for example is unable to trigger in MBF (and thus PrBoom+), yet the ports remains Vanilla compatible. |
I hope time for this comes soon :) |
@fabiangreffrath, I hope as the major hard-to-apply changes like Sigil 2 support have been implemented, maybe it's time for this? |
Well, one does not have anything to do with the other. The problem with the patch here is that it does not fix the underlying bug, but merely puts band-aid over its symptoms. However, there is a discussion currently going on in the DW forums about some code glitch which might be the reason for this bug, so there are chances that this may get fixed in a sane way. |
Could you please point me to it? |
|
Sure, the issue is discussed in detail here: In short words, A possible fix might be something like this, but "unfortunately" we are not allowed to change Vanilla Doom's game logic and so any other demo will desync with this change: --- a/src/doom/p_mobj.c
+++ b/src/doom/p_mobj.c
@@ -931,9 +931,11 @@ void P_CheckMissileSpawn (mobj_t* th)
// move a little forward so an angle can
// be computed if it immediately explodes
+ P_UnsetThingPosition (th);
th->x += (th->momx>>1);
th->y += (th->momy>>1);
th->z += (th->momz>>1);
+ P_SetThingPosition (th);
if (!P_TryMove (th, th->x, th->y))
P_ExplodeMissile (th); |
For blockmap please see the last 2 comments, maybe I can fix it in ROOM.DEH, how do I make the created BFG missile MF_NOBLOCKMAP? |
|
And indeed, this little change makes the freeze disappear: --- a/room.deh
+++ b/room_noblockmap.deh
@@ -34,7 +34,7 @@ Missile damage = 0
Height = 0
Initial frame = 118
Width = 0
-Bits = 67080
+Bits = 67096
Alert sound = 83
Speed = -65536
|
|
I'd consider this freeze consistent with Vanilla, the issue is not easy (impossible?) to fix in source code and a change of a mere number in the DEHACKED patch solves this issue for all ports and EXEs. |
|
Yes, nice that this bug turned out so simple to workaround in Dehacked! Re-uploaded my re-release of ROOM with this fix |
Background
Version of Crispy Doom: Chocolate Doom or any of its forks
Operating System and version: Windows 7 or 10
Game: Doom 2
Any loaded WADs and mods (please include full command line): ROOM https://www.moddb.com/mods/room/downloads/room-version-2
crispy-doom -iwad doom2.wad -merge room.wad -deh room.deh
more convenient for testing: so-doom -iwad doom2.wad -merge room.wad -deh room.deh
(in So Doom you can use HPS cheat to restore health, not sure if the bug is reproducible with IDDQD)
Bug description
Observed behavior: game sometimes freezes when Doomguy punches himself in the face.
@JNechaevsky says gdb doesn't throw any error, probably an infinite loop occurs.
The text was updated successfully, but these errors were encountered: