From 8962477cfa10850cb459d7de754c9a435cc91293 Mon Sep 17 00:00:00 2001 From: ThorstenB 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::const_iterator propIt; - std::vector::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 properties; + + ~FGExternalMotionData() + { + std::vector::const_iterator propIt; + std::vector::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::const_iterator propIt; - std::vector::const_iterator propItEnd; - propIt = motionInfo.properties.begin(); - propItEnd = motionInfo.properties.end(); - - //cout << "Deleting data\n"; - - while (propIt != propItEnd) - { - delete *propIt; - propIt++; - } } return true; -- 2.39.5