Memory leak fix for 1.28.1

source/P4VFS.Console/P4VFS.Notes.txt

@@ -1,5 +1,9 @@
 Microsoft P4VFS Release Notes
 
+Version [1.28.1.0]
+* Fixing possible memory leak during sync operation while accumulating logging 
+  elements for statistics
+
 Version [1.28.0.0]
 * Addition of 'dehydrate' command as synonym for 'resident -v', for changing existing
   files from #have to zero-byte placeholder files. Unit tests included.

source/P4VFS.Core/Include/LogDevice.h

@@ -174,16 +174,23 @@ namespace FileCore {
 
 	struct LogDeviceMemory : LogDevice
 	{
+		LogDeviceMemory();
+
 		virtual void Write(const LogElement& element) override;
+		const List<LogElement>& GetElements() const;
 
+	private:
 		List<LogElement> m_Elements;
+		AutoHandle m_ElementsMutex;
 	};
 
 	struct LogDeviceAggregate : LogDevice
 	{
 		virtual void Write(const LogElement& element) override;
 		virtual bool IsFaulted() override;
+		void AddDevice(LogDevice* device);
 
+	private:
 		Array<LogDevice*> m_Devices;
 	};
 
@@ -197,6 +204,7 @@ namespace FileCore {
 		virtual void Write(const LogElement& element) override;
 		virtual bool IsFaulted() override;
 
+	private:
 		LogDevice* m_InnerDevice;
 		LogChannel::Enum m_Level;
 	};

source/P4VFS.Core/Source/DepotOperations.cpp

@@ -152,8 +152,8 @@ DepotOperations::SyncVirtual(
 
 	if (depotClient->Log() != nullptr)
 	{
-		aggregateLog.m_Devices.push_back(depotClient->Log());
-		aggregateLog.m_Devices.push_back(&memoryLog);
+		aggregateLog.AddDevice(depotClient->Log());
+		aggregateLog.AddDevice(&memoryLog);
 		log = &aggregateLog;
 	}
 
@@ -479,7 +479,7 @@ DepotOperations::ApplyVirtualModification(
 	// Flush buffered log all at once
 	if (log == &memoryLog && parentLog != nullptr)
 	{
-		for (const LogElement& element : memoryLog.m_Elements)
+		for (const LogElement& element : memoryLog.GetElements())
 		{
 			parentLog->Write(element.m_Channel, element.m_Text);
 		}
@@ -749,8 +749,8 @@ DepotOperations::SyncRegular(
 
 	if (depotClient->Log() != nullptr)
 	{
-		aggregateLog.m_Devices.push_back(depotClient->Log());
-		aggregateLog.m_Devices.push_back(&memoryLog);
+		aggregateLog.AddDevice(depotClient->Log());
+		aggregateLog.AddDevice(&memoryLog);
 		log = &aggregateLog;
 	}
 
@@ -1430,10 +1430,10 @@ DepotOperations::LogWarningErrorSummary(
 		return e.m_Channel == LogChannel::Warning || e.m_Channel == LogChannel::Error;
 	};
 
-	if (Algo::Any(memoryLog.m_Elements, std::not_fn(inSummary)))
+	if (Algo::Any(memoryLog.GetElements(), std::not_fn(inSummary)))
 	{
 		bool writeHeader = true;
-		for (const LogElement& element : memoryLog.m_Elements)
+		for (const LogElement& element : memoryLog.GetElements())
 		{
 			if (inSummary(element))
 			{

source/P4VFS.Core/Source/DepotSyncAction.cpp

@@ -359,7 +359,7 @@ DepotSyncStatus::Enum DepotSyncStatus::FromLog(const LogDeviceMemory& log)
 {
 	StaticAssert(DepotSyncStatus::Success == 0);
 	DepotSyncStatus::Enum status = DepotSyncStatus::Success;
-	for (const LogElement& element : log.m_Elements)
+	for (const LogElement& element : log.GetElements())
 	{
 		status |= FromLogElement(element);
 	}

source/P4VFS.Core/Source/LogDevice.cpp

@@ -197,11 +197,22 @@ String LogDeviceFile::ExpandVariables(const String& text) const
 	return StringInfo::Replace(text.c_str(), VariableUserName, StringInfo::ToLower(GetDesiredUserName().c_str()).c_str(), StringInfo::SearchCase::Insensitive);
 }
 
+LogDeviceMemory::LogDeviceMemory() :
+	m_ElementsMutex(CreateMutex(NULL, FALSE, NULL))
+{
+}
+
 void LogDeviceMemory::Write(const LogElement& element)
 {
+	AutoMutex elementsScope(m_ElementsMutex);
 	m_Elements.push_back(element);
 }
 
+const List<LogElement>& LogDeviceMemory::GetElements() const
+{
+	return m_Elements;
+}
+
 void LogDeviceAggregate::Write(const LogElement& element)
 {
 	for (LogDevice* device : m_Devices)
@@ -225,6 +236,11 @@ bool LogDeviceAggregate::IsFaulted()
 	return false;
 }
 
+void LogDeviceAggregate::AddDevice(LogDevice* device)
+{
+	m_Devices.push_back(device);
+}
+
 LogDeviceFilter::LogDeviceFilter(LogDevice* device, LogChannel::Enum level) :
 	m_InnerDevice(device),
 	m_Level(level)