ZDoom

Fixed.
[quote="Blzut3"]However it is common practice for mirror polyobjs to have the source polyobj as it's mirror creating a cycle.[/quote]
I don't know if this statement is actually true now or not, but common practice in Hexen was that only one polyobject was marked with a mirror, so you wouldn't get cycles like this.

The loop is there to potentially allow more than 2 polyobjs to be chained together. (Polyobj 1 has polyobj 2 as a mirror and polyobj 2 has polyobj 3 as a mirror for example.) However it is common practice for mirror polyobjs to have the source polyobj as it's mirror creating a cycle. This normally isn't a problem as ZDoom will break the loop when it hits a polyobj that's already active, but when using the override functions it just cycles indefinitely.

I think this also points out another minor bug in the non-override functions. Since it breaks on the first mirror to have an active thinker. You could possibly jam the non-override functions when used on a non cyclic set of polyobjs. Haven't actually tried this though.

After that change the freeze is still there but it's a lot easier to stop the application. I still wonder why there's the loop (surely there's a missing assumption in the while loop).

[edit]Interestingly, 'mirror' oscillates between 2 and 1 ...

pe->Destroy() instead of delete pe;

In general any thinker object in the game may never plainly be 'delete'd.
You have to take care of internal references somehow and the destructor is not the correct spot for this. The object first needs to be garbage collected to ensure that it isn't referenced anymore.

Index: src/po_man.cpp
===================================================================
--- src/po_man.cpp	(revision 3399)
+++ src/po_man.cpp	(working copy)
@@ -618,7 +618,7 @@
 bool EV_MovePolyTo(line_t *line, int polyNum, int speed, fixed_t targx, fixed_t targy, bool overRide)
 {
 	int mirror;
-	DMovePolyTo *pe;
+	DMovePolyTo *pe = NULL;
 	FPolyObj *poly;
 	TVector2<double> dist;
 	double distlen;
@@ -652,6 +652,8 @@
 	{
 		pe->StopInterpolation();
 	}
+    delete pe;
+    pe = NULL;
 
 	while ( (mirror = poly->GetMirror()) )
 	{
@@ -677,6 +679,8 @@
 		{
 			pe->StopInterpolation();
 		}
+        delete pe;
+        pe = NULL;
 	}
 	return true;
 }

zdoom: /media/27522720-b102-4f44-b9ed-8a24d4ef0738/edward-san/zdoom/trunk/src/dthinker.cpp:229: virtual DThinker::~DThinker(): Assertion `NextThinker == __null && PrevThinker == __null' failed.

Hmm, I did this change:

[code]
Index: src/po_man.cpp
===================================================================
--- src/po_man.cpp (revision 3399)
+++ src/po_man.cpp (working copy)
@@ -618,7 +618,7 @@
bool EV_MovePolyTo(line_t *line, int polyNum, int speed, fixed_t targx, fixed_t targy, bool overRide)
{
int mirror;
- DMovePolyTo *pe;
+ DMovePolyTo *pe = NULL;
FPolyObj *poly;
TVector2<double> dist;
double distlen;
@@ -652,6 +652,8 @@
{
pe->StopInterpolation();
}
+ delete pe;
+ pe = NULL;

while ( (mirror = poly->GetMirror()) )
{
@@ -677,6 +679,8 @@
{
pe->StopInterpolation();
}
+ delete pe;
+ pe = NULL;
}
return true;
}
[/code]

Now in debug mode I have a failed assertion when I push the button:
[code]
zdoom: /media/27522720-b102-4f44-b9ed-8a24d4ef0738/edward-san/zdoom/trunk/src/dthinker.cpp:229: virtual DThinker::~DThinker(): Assertion `NextThinker == __null && PrevThinker == __null' failed.
[/code]
seems like 'pe' can't be deleted normally. How is it done?

[quote="Blzut3"]appears to be[b][size=150] line 666[/size][/b] of po_man.cpp[/quote]
:lol:

Thanks for this analysis. I expected something along these lines.

	while ( (mirror = poly->GetMirror()) )
	{
		poly = PO_GetPolyobj(mirror);
		if (poly == NULL || (poly->specialdata != NULL && !overRide))
		{ // mirroring poly does not exist or is already in motion
			break;
		}
		// reverse the direction
		dist.X = -dist.X;
		dist.Y = -dist.Y;
		pe = new DMovePolyTo(mirror);
		poly->specialdata = pe;
		pe->m_Dist = xs_RoundToInt(distlen);
		pe->m_Speed = speed;
		pe->m_xSpeed = xs_RoundToInt(speed * dist.X);
		pe->m_ySpeed = xs_RoundToInt(speed * dist.Y);
		pe->m_xTarget = xs_RoundToInt(poly->StartSpot.x + distlen * dist.X);
		pe->m_yTarget = xs_RoundToInt(poly->StartSpot.y + distlen * dist.Y);
		polyNum = mirror;
		SN_StartSequence(poly, poly->seqType, SEQ_DOOR, 0);
		if (nointerp)
		{
			pe->StopInterpolation();
		}
	}

The "leak" (not really a leak since the object will eventually get cleaned up) appears to be line 666 of po_man.cpp:
[code] while ( (mirror = poly->GetMirror()) )
{
poly = PO_GetPolyobj(mirror);
if (poly == NULL || (poly->specialdata != NULL && !overRide))
{ // mirroring poly does not exist or is already in motion
break;
}
// reverse the direction
dist.X = -dist.X;
dist.Y = -dist.Y;
pe = new DMovePolyTo(mirror);
poly->specialdata = pe;
pe->m_Dist = xs_RoundToInt(distlen);
pe->m_Speed = speed;
pe->m_xSpeed = xs_RoundToInt(speed * dist.X);
pe->m_ySpeed = xs_RoundToInt(speed * dist.Y);
pe->m_xTarget = xs_RoundToInt(poly->StartSpot.x + distlen * dist.X);
pe->m_yTarget = xs_RoundToInt(poly->StartSpot.y + distlen * dist.Y);
polyNum = mirror;
SN_StartSequence(poly, poly->seqType, SEQ_DOOR, 0);
if (nointerp)
{
pe->StopInterpolation();
}
}[/code]
[s]I'm not an expert on this piece of code, but having a loop there seems wrong to me. Doesn't FPolyObj::GetMirror() only return one possible value (Linedefs[0]->args[1])?[/s] If this is the case then I think every override function will lock up if the poly object has a mirror.

Edit: Actually on a second look I see where poly changes (first line of the loop of course). So the problem is each poly being the mirror of the other so the loop just flips between the two indefinitely.

[quote="terranova"]I'm probably going to sound stupid by saying this, but anyone tried to test this after applying the 4GB RAM patch on ZD/GZD?[/quote]
It's a case of an infinite loop. So on my 64-bit system it promptly ate all 8GB and started into the swap space as well. On a side note, valgrind isn't working with ZDoom for some reason.

I haven't run it in the debugger yet but it's plainly obvious that the code gets stuck in an endless loop that repeatedly allocates some memory. It would exceed any amount that could be set.

[quote="terranova"]I'm probably going to sound stupid by saying this, but anyone tried to test this after applying the 4GB RAM patch on ZD/GZD?[/quote]
If a memory leaks gobbles up two gigs of RAM instantly, it'll gobble up 4 gigs of RAM instantly as well.

A slow memory leak, like maybe a megabyte every hour or so, well then, no problem. You can suggest workarounds that don't fix the issue but only alleviate the symptoms. A lightning fast memory leak, however, won't be assuaged just by giving it double the amount of memory. Maybe if you gave it a billion yottabytes, but that's not possible.

[quote="TheDarkArchon"]Interestingly, when I checked how much RAM GZDoom was using at the point it went boom, it was "only" using 1.89GB of RAM (I.E it hadn't completely swallowed the 2GB limit allowed by 32-bit programs in Windows).

EDIT: ZDoom r3394 went up to 1.91GB of RAM before moaning about it's inability to chomp through more RAM.[/quote]

[quote="Enjay"]I just checked on my system and it reported much the same memory use. I ran Zdoom on one screen and the task manager was visible on the other. When I tried to use the polyobject, the memory use went up in approximately 400MB jumps (slightly quicker than about 1 jump per second) 'til it got to around 1.9GB then it fluctuated up and down a bit around that value until I pressed PrtScrn. At that point I was dropped back to the desktop with the ZDoom console window showing the malloc error.

[img]http://i.imgur.com/S5sam.png[/img][/quote]

That is one hell of a memory leak :shock:

I'm probably going to sound stupid by saying this, but anyone tried to test this after applying the 4GB RAM patch on ZD/GZD?

I just checked on my system and it reported much the same memory use. I ran Zdoom on one screen and the task manager was visible on the other. When I tried to use the polyobject, the memory use went up in approximately 400MB jumps (slightly quicker than about 1 jump per second) 'til it got to around 1.9GB then it fluctuated up and down a bit around that value until I pressed PrtScrn. At that point I was dropped back to the desktop with the ZDoom console window showing the malloc error.

[img]http://i.imgur.com/S5sam.png[/img]

Interestingly, when I checked how much RAM GZDoom was using at the point it went boom, it was "only" using 1.89GB of RAM (I.E it hadn't completely swallowed the 2GB limit allowed by 32-bit programs in Windows).

EDIT: ZDoom r3394 went up to 1.91GB of RAM before moaning about it's inability to chomp through more RAM.

FWiW, r3399 still locks up for me with this map and I still get the "Could not malloc 60 bytes" error when I manage to force my way back to the desktop.