From 8962477cfa10850cb459d7de754c9a435cc91293 Mon Sep 17 00:00:00 2001
From: ThorstenB <brehmt@gmail.com>
Date: Wed, 16 Feb 2011 00:49:00 +0100
Subject: [PATCH] Fix huge multiplayer memory leak. Almost all FGPropertyData
elements received via MP were leaked. Property data is now cleanly
deallocated in the FGExternalMotionData destructor. Thanks to Jester for
reporting rising mem consumption in MP mode.
---
src/AIModel/AIMultiplayer.cxx | 29 +++++------------------------
src/AIModel/AIMultiplayer.hxx | 2 +-
src/MultiPlayer/mpmessages.hxx | 16 +++++++++++++++-
src/MultiPlayer/multiplaymgr.cxx | 11 ++++++-----
src/Network/multiplay.cxx | 14 --------------
5 files changed, 27 insertions(+), 45 deletions(-)
diff --git a/src/AIModel/AIMultiplayer.cxx b/src/AIModel/AIMultiplayer.cxx
index 476ddf205..0652dfef7 100644
--- a/src/AIModel/AIMultiplayer.cxx
+++ b/src/AIModel/AIMultiplayer.cxx
@@ -335,35 +335,13 @@ void FGAIMultiplayer::update(double dt)
if (prevIt != mMotionInfo.begin())
{
--prevIt;
-
- MotionInfo::iterator delIt;
- delIt = mMotionInfo.begin();
-
- while (delIt != prevIt)
- {
- std::vector<FGPropertyData*>::const_iterator propIt;
- std::vector<FGPropertyData*>::const_iterator propItEnd;
- propIt = delIt->second.properties.begin();
- propItEnd = delIt->second.properties.end();
-
- //cout << "Deleting data\n";
-
- while (propIt != propItEnd)
- {
- delete *propIt;
- propIt++;
- }
-
- delIt++;
- }
-
mMotionInfo.erase(mMotionInfo.begin(), prevIt);
}
}
} else {
// Ok, we need to predict the future, so, take the best data we can have
// and do some eom computation to guess that for now.
- FGExternalMotionData motionInfo = it->second;
+ FGExternalMotionData& motionInfo = it->second;
// The time to predict, limit to 5 seconds
double t = tInterp - motionInfo.time;
@@ -488,7 +466,7 @@ void FGAIMultiplayer::update(double dt)
}
void
-FGAIMultiplayer::addMotionInfo(const FGExternalMotionData& motionInfo,
+FGAIMultiplayer::addMotionInfo(FGExternalMotionData& motionInfo,
long stamp)
{
mLastTimestamp = stamp;
@@ -505,6 +483,9 @@ FGAIMultiplayer::addMotionInfo(const FGExternalMotionData& motionInfo,
return;
}
mMotionInfo[motionInfo.time] = motionInfo;
+ // We just copied the property (pointer) list - they are ours now. Clear the
+ // properties list in given/returned object, so former owner won't deallocate them.
+ motionInfo.properties.clear();
}
void
diff --git a/src/AIModel/AIMultiplayer.hxx b/src/AIModel/AIMultiplayer.hxx
index e484dacb0..d79f0c206 100644
--- a/src/AIModel/AIMultiplayer.hxx
+++ b/src/AIModel/AIMultiplayer.hxx
@@ -37,7 +37,7 @@ public:
virtual void unbind();
virtual void update(double dt);
- void addMotionInfo(const FGExternalMotionData& motionInfo, long stamp);
+ void addMotionInfo(FGExternalMotionData& motionInfo, long stamp);
void setDoubleProperty(const std::string& prop, double val);
long getLastTimestamp(void) const
diff --git a/src/MultiPlayer/mpmessages.hxx b/src/MultiPlayer/mpmessages.hxx
index dded2b3e2..f5b89eb82 100644
--- a/src/MultiPlayer/mpmessages.hxx
+++ b/src/MultiPlayer/mpmessages.hxx
@@ -141,7 +141,7 @@ struct FGExternalMotionData {
// simulation time when this packet was generated
double time;
// the artificial lag the client should stay behind the average
- // simulation time to arrival time diference
+ // simulation time to arrival time difference
// FIXME: should be some 'per model' instead of 'per packet' property
double lag;
@@ -166,6 +166,20 @@ struct FGExternalMotionData {
// The set of properties recieved for this timeslot
std::vector<FGPropertyData*> properties;
+
+ ~FGExternalMotionData()
+ {
+ std::vector<FGPropertyData*>::const_iterator propIt;
+ std::vector<FGPropertyData*>::const_iterator propItEnd;
+ propIt = properties.begin();
+ propItEnd = properties.end();
+
+ while (propIt != propItEnd)
+ {
+ delete *propIt;
+ propIt++;
+ }
+ }
};
#endif
diff --git a/src/MultiPlayer/multiplaymgr.cxx b/src/MultiPlayer/multiplaymgr.cxx
index 7657ad4ee..9a5413827 100644
--- a/src/MultiPlayer/multiplaymgr.cxx
+++ b/src/MultiPlayer/multiplaymgr.cxx
@@ -923,19 +923,20 @@ FGMultiplayMgr::ProcessPosMsg(const FGMultiplayMgr::MsgBuf& Msg,
goto noprops;
}
while (xdr < Msg.propsRecvdEnd()) {
- FGPropertyData* pData = new FGPropertyData;
// simgear::props::Type type = simgear::props::UNSPECIFIED;
// First element is always the ID
- pData->id = XDR_decode_uint32(*xdr);
+ unsigned id = XDR_decode_uint32(*xdr);
//cout << pData->id << " ";
xdr++;
// Check the ID actually exists and get the type
- const IdPropertyList* plist = findProperty(pData->id);
+ const IdPropertyList* plist = findProperty(id);
if (plist)
{
+ FGPropertyData* pData = new FGPropertyData;
+ pData->id = id;
pData->type = plist->type;
// How we decode the remainder of the property depends on the type
switch (pData->type) {
@@ -1001,7 +1002,7 @@ FGMultiplayMgr::ProcessPosMsg(const FGMultiplayMgr::MsgBuf& Msg,
// We failed to find the property. We'll try the next packet immediately.
SG_LOG(SG_NETWORK, SG_INFO, "FGMultiplayMgr::ProcessPosMsg - "
"message from " << MsgHdr->Callsign << " has unknown property id "
- << pData->id);
+ << id);
}
}
noprops:
@@ -1015,7 +1016,7 @@ FGMultiplayMgr::ProcessPosMsg(const FGMultiplayMgr::MsgBuf& Msg,
//////////////////////////////////////////////////////////////////////
//
// handle a chat message
-// FIXME: display chat message withi flightgear
+// FIXME: display chat message within flightgear
//
//////////////////////////////////////////////////////////////////////
void
diff --git a/src/Network/multiplay.cxx b/src/Network/multiplay.cxx
index a9cd32bf3..ddedccea8 100644
--- a/src/Network/multiplay.cxx
+++ b/src/Network/multiplay.cxx
@@ -283,20 +283,6 @@ bool FGMultiplay::process() {
FGMultiplayMgr* mpmgr = (FGMultiplayMgr*) globals->get_subsystem("mp");
mpmgr->SendMyPosition(motionInfo);
-
- // Now remove the data
- std::vector<FGPropertyData*>::const_iterator propIt;
- std::vector<FGPropertyData*>::const_iterator propItEnd;
- propIt = motionInfo.properties.begin();
- propItEnd = motionInfo.properties.end();
-
- //cout << "Deleting data\n";
-
- while (propIt != propItEnd)
- {
- delete *propIt;
- propIt++;
- }
}
return true;