From 02813113e27e9fade3ae1b5cd2f6fe56118c89f1 Mon Sep 17 00:00:00 2001
From: Richard Kimberly Heck <rikiheck@lyx.org>
Date: Sun, 23 Feb 2020 16:39:00 -0500
Subject: [PATCH] Try again to fix memory leak reported by Scott.

This time, use a shared_ptr.
---
 src/Buffer.cpp | 24 +++++++++++++-----------
 src/Buffer.h   |  3 ++-
 2 files changed, 15 insertions(+), 12 deletions(-)

diff --git a/src/Buffer.cpp b/src/Buffer.cpp
index eb65e811b4..d78b5f49bd 100644
--- a/src/Buffer.cpp
+++ b/src/Buffer.cpp
@@ -154,7 +154,8 @@ typedef vector<LabelInfo> LabelCache;
 typedef map<docstring, Buffer::References> RefCache;
 
 // A storehouse for the cloned buffers.
-std::list<CloneList *> cloned_buffers;
+typedef list<CloneList_ptr> CloneStore;
+CloneStore cloned_buffers;
 
 } // namespace
 
@@ -371,7 +372,7 @@ public:
 	/// This one is useful for preview detached in a thread.
 	Buffer const * cloned_buffer_;
 	///
-	CloneList * clone_list_;
+	CloneList_ptr clone_list_;
 	/// are we in the process of exporting this buffer?
 	mutable bool doing_export;
 
@@ -530,8 +531,8 @@ Buffer::~Buffer()
 		// loop over children
 		for (auto const & p : d->children_positions) {
 			Buffer * child = const_cast<Buffer *>(p.first);
-				if (d->clone_list_->erase(child))
-					delete child;
+			if (d->clone_list_->erase(child))
+				delete child;
 		}
 		// if we're the master buffer, then we should get rid of the list
 		// of clones
@@ -540,14 +541,15 @@ Buffer::~Buffer()
 			// children still has a reference to this list. But we will try to
 			// continue, rather than shut down.
 			LATTEST(d->clone_list_->empty());
-			list<CloneList *>::iterator it =
+			// The clone list itself is empty, but it's still referenced in our list
+			// of clones. So let's find it and remove it.
+			CloneStore::iterator it =
 				find(cloned_buffers.begin(), cloned_buffers.end(), d->clone_list_);
 			if (it == cloned_buffers.end()) {
 				// We will leak in this case, but it is safe to continue.
 				LATTEST(false);
 			} else
 				cloned_buffers.erase(it);
-			delete d->clone_list_;
 		}
 		// FIXME Do we really need to do this right before we delete d?
 		// clear references to children in macro tables
@@ -594,8 +596,8 @@ Buffer::~Buffer()
 Buffer * Buffer::cloneWithChildren() const
 {
 	BufferMap bufmap;
-	cloned_buffers.push_back(new CloneList);
-	CloneList * clones = cloned_buffers.back();
+	cloned_buffers.push_back(CloneList_ptr(new CloneList));
+	CloneList_ptr clones = cloned_buffers.back();
 
 	cloneWithChildren(bufmap, clones);
 
@@ -608,7 +610,7 @@ Buffer * Buffer::cloneWithChildren() const
 }
 
 
-void Buffer::cloneWithChildren(BufferMap & bufmap, CloneList * clones) const
+void Buffer::cloneWithChildren(BufferMap & bufmap, CloneList_ptr clones) const
 {
 	// have we already been cloned?
 	if (bufmap.find(this) != bufmap.end())
@@ -658,8 +660,8 @@ void Buffer::cloneWithChildren(BufferMap & bufmap, CloneList * clones) const
 
 
 Buffer * Buffer::cloneBufferOnly() const {
-	cloned_buffers.push_back(new CloneList);
-	CloneList * clones = cloned_buffers.back();
+	cloned_buffers.push_back(CloneList_ptr(new CloneList));
+	CloneList_ptr clones = cloned_buffers.back();
 	Buffer * buffer_clone = new Buffer(fileName().absFileName(), false, this);
 
 	// The clone needs its own DocumentClass, since running updateBuffer() will
diff --git a/src/Buffer.h b/src/Buffer.h
index 04e9d60518..4d27ec3c01 100644
--- a/src/Buffer.h
+++ b/src/Buffer.h
@@ -82,6 +82,7 @@ class Buffer;
 typedef std::list<Buffer *> ListOfBuffers;
 /// a list of Buffers we cloned
 typedef std::set<Buffer *> CloneList;
+typedef std::shared_ptr<CloneList> CloneList_ptr;
 
 
 /** The buffer object.
@@ -231,7 +232,7 @@ private:
 	///
 	typedef std::map<Buffer const *, Buffer *> BufferMap;
 	///
-	void cloneWithChildren(BufferMap &, CloneList *) const;
+	void cloneWithChildren(BufferMap &, CloneList_ptr) const;
 	/// save checksum of the given file.
 	void saveCheckSum() const;
 	/// read a new file
-- 
2.39.5