anne/lyx_mirror
1
0
Fork 0
mirror of https://git.lyx.org/repos/lyx.git synced 2024-11-09 18:31:04 +00:00
Code Issues Actions Packages Projects Releases Wiki Activity

Deal with memory issue reported some time ago in connection with DocumentClass

Browse Source 
objects. The problem that led to the leak is that these objects can be held in
memory long after the Buffer that created them is gone, mostly due to their
use in the CutStack. So they were previously held in a storage facility, the
DocumentClassBundle. Unfortunately, they were now being created too often,
especially by cloning. It's not really a leak, because they're accessible, but
we weren't ever destroying them.

This new approach uses a shared_ptr instead.

Thanks to Vincent for pointing out const_pointer_cast.
This commit is contained in:
Richard Heck 2012-03-13 12:13:31 -04:00
parent 03da96e0f7
commit ead697d4b6
12 changed files with 79 additions and 103 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

10
src/BufferParams.cpp
View File

@ -1953,20 +1953,20 @@ bool BufferParams::hasClassDefaults() const
DocumentClass const & BufferParams::documentClass() const
{
return *doc_class_;
return *doc_class_.get();
}
DocumentClass const * BufferParams::documentClassPtr() const
DocumentClassConstPtr BufferParams::documentClassPtr() const
{
return doc_class_;
}
void BufferParams::setDocumentClass(DocumentClass const * const tc)
void BufferParams::setDocumentClass(DocumentClassConstPtr tc)
{
// evil, but this function is evil
doc_class_ = const_cast<DocumentClass *>(tc);
doc_class_ = const_pointer_cast<DocumentClass>(tc);
}
@ -2037,7 +2037,7 @@ void BufferParams::makeDocumentClass()
en = cite_engine_.end();
for (; it != en; ++it)
mods.push_back(*it);
doc_class_ = &(DocumentClassBundle::get().makeDocumentClass(*baseClass(), mods));
doc_class_ = getDocumentClass(*baseClass(), mods);
if (!local_layout.empty()) {
TextClass::ReturnValues success =

7
src/BufferParams.h
View File

@ -16,6 +16,7 @@
#define BUFFERPARAMS_H
#include "Citation.h"
#include "DocumentClassPtr.h"
#include "Format.h"
#include "LayoutModuleList.h"
#include "OutputParams.h"
@ -129,11 +130,11 @@ public:
DocumentClass const & documentClass() const;
/// \return A pointer to the DocumentClass currently in use: the BaseClass
/// as modified by modules.
DocumentClass const * documentClassPtr() const;
DocumentClassConstPtr documentClassPtr() const;
/// This bypasses the baseClass and sets the textClass directly.
/// Should be called with care and would be better not being here,
/// but it seems to be needed by CutAndPaste::putClipboard().
void setDocumentClass(DocumentClass const * const);
void setDocumentClass(DocumentClassConstPtr);
/// List of modules in use
LayoutModuleList const & getModules() const { return layout_modules_; }
/// List of default modules the user has removed
@ -500,7 +501,7 @@ private:
/// the type of cite engine (authoryear or numerical)
CiteEngineType cite_engine_type_;
///
DocumentClass * doc_class_;
DocumentClassPtr doc_class_;
///
LayoutModuleList layout_modules_;
/// this is for modules that are required by the document class but that

14
src/BufferView.cpp
View File

@ -941,7 +941,7 @@ bool BufferView::scrollToCursor(DocIterator const & dit, bool recenter)
}
void BufferView::updateDocumentClass(DocumentClass const * const olddc)
void BufferView::updateDocumentClass(DocumentClassConstPtr olddc)
{
message(_("Converting document to new document class..."));
@ -1234,7 +1234,7 @@ void BufferView::dispatch(FuncRequest const & cmd, DispatchResult & dr)
switch (act) {
case LFUN_BUFFER_PARAMS_APPLY: {
DocumentClass const * const oldClass = buffer_.params().documentClassPtr();
DocumentClassConstPtr oldClass = buffer_.params().documentClassPtr();
cur.recordUndoFullDocument();
istringstream ss(to_utf8(cmd.argument()));
Lexer lex;
@ -1256,8 +1256,7 @@ void BufferView::dispatch(FuncRequest const & cmd, DispatchResult & dr)
}
case LFUN_LAYOUT_MODULES_CLEAR: {
DocumentClass const * const oldClass =
buffer_.params().documentClassPtr();
DocumentClassConstPtr oldClass = buffer_.params().documentClassPtr();
cur.recordUndoFullDocument();
buffer_.params().clearLayoutModules();
buffer_.params().makeDocumentClass();
@ -1275,7 +1274,7 @@ void BufferView::dispatch(FuncRequest const & cmd, DispatchResult & dr)
"conflicts with installed modules.");
break;
}
DocumentClass const * const oldClass = params.documentClassPtr();
DocumentClassConstPtr oldClass = params.documentClassPtr();
cur.recordUndoFullDocument();
buffer_.params().addLayoutModule(argument);
buffer_.params().makeDocumentClass();
@ -1306,8 +1305,7 @@ void BufferView::dispatch(FuncRequest const & cmd, DispatchResult & dr)
break;
// Save the old, possibly modular, layout for use in conversion.
DocumentClass const * const oldDocClass =
buffer_.params().documentClassPtr();
DocumentClassConstPtr oldDocClass = buffer_.params().documentClassPtr();
cur.recordUndoFullDocument();
buffer_.params().setBaseClass(argument);
buffer_.params().makeDocumentClass();
@ -1332,7 +1330,7 @@ void BufferView::dispatch(FuncRequest const & cmd, DispatchResult & dr)
}
case LFUN_LAYOUT_RELOAD: {
DocumentClass const * const oldClass = buffer_.params().documentClassPtr();
DocumentClassConstPtr oldClass = buffer_.params().documentClassPtr();
LayoutFileIndex bc = buffer_.params().baseClassID();
LayoutFileList::get().reset(bc);
buffer_.params().setBaseClass(bc);

4
src/BufferView.h
View File

@ -15,8 +15,10 @@
#ifndef BUFFER_VIEW_H
#define BUFFER_VIEW_H
#include "DocumentClassPtr.h"
#include "update_flags.h"
#include "support/shared_ptr.h"
#include "support/strfwd.h"
#include "support/types.h"
@ -345,7 +347,7 @@ private:
void updateHoveredInset() const;
///
void updateDocumentClass(DocumentClass const * const olddc);
void updateDocumentClass(DocumentClassConstPtr olddc);
///
int width_;
///

29
src/CutAndPaste.cpp
View File

@ -79,7 +79,7 @@ namespace {
typedef pair<pit_type, int> PitPosPair;
typedef limited_stack<pair<ParagraphList, DocumentClass const *> > CutStack;
typedef limited_stack<pair<ParagraphList, DocumentClassConstPtr> > CutStack;
CutStack theCuts(10);
// persistent selection, cleared until the next selection
@ -99,7 +99,7 @@ bool checkPastePossible(int index)
pair<PitPosPair, pit_type>
pasteSelectionHelper(Cursor const & cur, ParagraphList const & parlist,
DocumentClass const * const oldDocClass, ErrorList & errorlist)
DocumentClassConstPtr oldDocClass, ErrorList & errorlist)
{
Buffer const & buffer = *cur.buffer();
pit_type pit = cur.pit();
@ -119,8 +119,7 @@ pasteSelectionHelper(Cursor const & cur, ParagraphList const & parlist,
// Make a copy of the CaP paragraphs.
ParagraphList insertion = parlist;
DocumentClass const * const newDocClass =
buffer.params().documentClassPtr();
DocumentClassConstPtr newDocClass = buffer.params().documentClassPtr();
// Now remove all out of the pars which is NOT allowed in the
// new environment and set also another font if that is required.
@ -461,15 +460,14 @@ PitPosPair eraseSelectionHelper(BufferParams const & params,
void putClipboard(ParagraphList const & paragraphs,
DocumentClass const * const docclass, docstring const & plaintext)
DocumentClassConstPtr docclass, docstring const & plaintext)
{
// For some strange reason gcc 3.2 and 3.3 do not accept
// Buffer buffer(string(), false);
// This needs to be static to avoid a memory leak. When a Buffer is
// constructed, it constructs a BufferParams, which in turn constructs
// a DocumentClass, via new, that is never deleted. If we were to go to
// some kind of garbage collection there, or a shared_ptr, then this
// would not be needed.
// This used to need to be static to avoid a memory leak. It no longer needs
// to be so, but the alternative is to construct a new one of these (with a
// new temporary directory, etc) every time, and then to destroy it. So maybe
// it's worth just keeping this one around.
static Buffer * buffer = theBufferList().newInternalBuffer(
FileName::tempName("clipboard.internal").absFileName());
buffer->setUnnamed(true);
@ -503,7 +501,7 @@ static bool isFullyDeleted(ParagraphList const & pars)
void copySelectionHelper(Buffer const & buf, Text const & text,
pit_type startpit, pit_type endpit,
int start, int end, DocumentClass const * const dc, CutStack & cutstack)
int start, int end, DocumentClassConstPtr dc, CutStack & cutstack)
{
ParagraphList const & pars = text.paragraphs();
@ -558,8 +556,7 @@ void copySelectionHelper(Buffer const & buf, Text const & text,
it->setInsetOwner(0);
}
DocumentClass * d = const_cast<DocumentClass *>(dc);
cutstack.push(make_pair(copy_pars, d));
cutstack.push(make_pair(copy_pars, dc));
}
} // namespace anon
@ -634,8 +631,8 @@ bool multipleCellsSelected(Cursor const & cur)
}
void switchBetweenClasses(DocumentClass const * const oldone,
DocumentClass const * const newone, InsetText & in, ErrorList & errorlist)
void switchBetweenClasses(DocumentClassConstPtr oldone,
DocumentClassConstPtr newone, InsetText & in, ErrorList & errorlist)
{
errorlist.clear();
@ -975,7 +972,7 @@ docstring selection(size_t sel_index)
void pasteParagraphList(Cursor & cur, ParagraphList const & parlist,
DocumentClass const * const docclass, ErrorList & errorList)
DocumentClassConstPtr docclass, ErrorList & errorList)
{
if (cur.inTexted()) {
Text * text = cur.text();

8
src/CutAndPaste.h
View File

@ -14,6 +14,8 @@
#ifndef CUTANDPASTE_H
#define CUTANDPASTE_H
#include "DocumentClassPtr.h"
#include "support/docstring.h"
#include "frontends/Clipboard.h"
@ -100,15 +102,15 @@ void pasteSimpleText(Cursor & cur, bool asParagraphs);
/// Paste the paragraph list \p parlist at the position given by \p cur.
/// Does not handle undo. Does only work in text, not mathed.
void pasteParagraphList(Cursor & cur, ParagraphList const & parlist,
DocumentClass const * const textclass, ErrorList & errorList);
DocumentClassConstPtr textclass, ErrorList & errorList);
/** Needed to switch between different classes. This works
* for a list of paragraphs beginning with the specified par.
* It changes layouts and character styles.
*/
void switchBetweenClasses(DocumentClass const * const c1,
DocumentClass const * const c2, InsetText & in, ErrorList &);
void switchBetweenClasses(DocumentClassConstPtr c1,
DocumentClassConstPtr c2, InsetText & in, ErrorList &);
/// Get the current selection as a string. Does not change the selection.
/// Does only work if the whole selection is in mathed.

24
src/DocumentClassPtr.h Normal file
View File

@ -0,0 +1,24 @@
// -*- C++ -*-
/**
* \file DocumentClassPtr.h
* This file is part of LyX, the document processor.
* Licence details can be found in the file COPYING.
*
* \author Richard Heck
*
* Full author contact details are available in file CREDITS.
*/
#ifndef DOCUMENT_CLASS_PTR_H
#define DOCUMENT_CLASS_PTR_H
#include "support/shared_ptr.h"
namespace lyx {
class DocumentClass;
typedef shared_ptr<DocumentClass> DocumentClassPtr;
typedef shared_ptr<DocumentClass const> DocumentClassConstPtr;
}
#endif // DISPATCH_RESULT_H

1
src/Makefile.am
View File

@ -216,6 +216,7 @@ HEADERFILESCORE = \
DepTable.h \
DispatchResult.h \
DocIterator.h \
DocumentClassPtr.h \
Encoding.h \
ErrorList.h \
Exporter.h \

35
src/TextClass.cpp
View File

@ -1431,38 +1431,11 @@ Layout TextClass::createBasicLayout(docstring const & name, bool unknown) const
}
/////////////////////////////////////////////////////////////////////////
//
// DocumentClassBundle
//
/////////////////////////////////////////////////////////////////////////
DocumentClassBundle::~DocumentClassBundle()
{
for (size_t i = 0; i != documentClasses_.size(); ++i)
delete documentClasses_[i];
documentClasses_.clear();
}
DocumentClass & DocumentClassBundle::newClass(LayoutFile const & baseClass)
{
DocumentClass * dc = new DocumentClass(baseClass);
documentClasses_.push_back(dc);
return *documentClasses_.back();
}
DocumentClassBundle & DocumentClassBundle::get()
{
static DocumentClassBundle singleton;
return singleton;
}
DocumentClass & DocumentClassBundle::makeDocumentClass(
DocumentClassPtr getDocumentClass(
LayoutFile const & baseClass, LayoutModuleList const & modlist)
{
DocumentClass & doc_class = newClass(baseClass);
DocumentClassPtr doc_class =
DocumentClassPtr(new DocumentClass(baseClass));
LayoutModuleList::const_iterator it = modlist.begin();
LayoutModuleList::const_iterator en = modlist.end();
for (; it != en; ++it) {
@ -1490,7 +1463,7 @@ DocumentClass & DocumentClassBundle::makeDocumentClass(
frontend::Alert::warning(_("Package not available"), msg, true);
}
FileName layout_file = libFileSearch("layouts", lm->getFilename());
if (!doc_class.read(layout_file, TextClass::MODULE)) {
if (!doc_class->read(layout_file, TextClass::MODULE)) {
docstring const msg =
bformat(_("Error reading module %1$s\n"), from_utf8(modName));
frontend::Alert::warning(_("Read Error"), msg);

41
src/TextClass.h
View File

@ -13,6 +13,7 @@
#include "Citation.h"
#include "ColorCode.h"
#include "Counters.h"
#include "DocumentClassPtr.h"
#include "FloatList.h"
#include "FontInfo.h"
#include "Layout.h"
@ -24,8 +25,6 @@
#include "support/docstring.h"
#include "support/types.h"
#include <boost/noncopyable.hpp>
#include <list>
#include <map>
#include <set>
@ -369,7 +368,7 @@ private:
/// In the main LyX code, DocumentClass objects are created only by
/// DocumentClassBundle, for which see below.
///
class DocumentClass : public TextClass, boost::noncopyable {
class DocumentClass : public TextClass {
public:
///
virtual ~DocumentClass() {}
@ -475,41 +474,19 @@ protected:
private:
/// The only class that can create a DocumentClass is
/// DocumentClassBundle, which calls the protected constructor.
friend class DocumentClassBundle;
friend DocumentClassPtr
getDocumentClass(LayoutFile const &, LayoutModuleList const &);
///
static InsetLayout plain_insetlayout_;
};
/// DocumentClassBundle is a container for DocumentClass objects, so that
/// they stay in memory for use by Insets, CutAndPaste, and the like, even
/// when their associated Buffers are destroyed.
/// FIXME Some sort of garbage collection or reference counting wouldn't
/// be a bad idea here. It might be enough to check when a Buffer is closed
/// (or makeDocumentClass is called) whether the old DocumentClass is in use
/// anywhere.
///
/// This is a singleton class. Its sole instance is accessed via
/// DocumentClassBundle::get().
class DocumentClassBundle : boost::noncopyable {
public:
/// \return The sole instance of this class.
static DocumentClassBundle & get();
/// \return A new DocumentClass based on baseClass, with info added
/// from the modules in modlist.
DocumentClass & makeDocumentClass(LayoutFile const & baseClass,
/// The only way to make a DocumentClass is to call this function.
/// The shared_ptr is needed because DocumentClass objects can be kept
/// in memory long after their associated Buffer is destroyed, mostly
/// on the CutStack.
DocumentClassPtr getDocumentClass(LayoutFile const & baseClass,
LayoutModuleList const & modlist);
private:
/// control instantiation
DocumentClassBundle() {}
/// clean up
~DocumentClassBundle();
/// \return Reference to a new DocumentClass equal to baseClass
DocumentClass & newClass(LayoutFile const & baseClass);
///
std::vector<DocumentClass *> documentClasses_;
};
/// convert page sides option to text 1 or 2
std::ostream & operator<<(std::ostream & os, PageSides p);

2
src/support/shared_ptr.h
View File

@ -23,6 +23,7 @@
namespace lyx
{
using std::tr1::shared_ptr;
using std::tr1::const_pointer_cast;
}
#else
@ -32,6 +33,7 @@ namespace lyx
namespace lyx
{
using boost::shared_ptr;
using boost::const_pointer_cast;
}
#endif

7
src/tex2lyx/tex2lyx.cpp
View File

@ -254,12 +254,11 @@ bool checkModule(string const & name, bool command)
// This is needed since a module cannot be read on its own, only as
// part of a document class.
LayoutFile const & baseClass = LayoutFileList::get()[textclass.name()];
typedef map<string, DocumentClass *> ModuleMap;
typedef map<string, DocumentClassPtr > ModuleMap;
static ModuleMap modules;
static bool init = true;
if (init) {
baseClass.load();
DocumentClassBundle & bundle = DocumentClassBundle::get();
LyXModuleList::const_iterator const end = theModuleList.end();
LyXModuleList::const_iterator it = theModuleList.begin();
for (; it != end; ++it) {
@ -269,7 +268,7 @@ bool checkModule(string const & name, bool command)
if (!m.moduleCanBeAdded(module, &baseClass))
continue;
m.push_back(module);
modules[module] = &bundle.makeDocumentClass(baseClass, m);
modules[module] = getDocumentClass(baseClass, m);
}
init = false;
}
@ -290,7 +289,7 @@ bool checkModule(string const & name, bool command)
continue;
if (findInsetLayoutWithoutModule(textclass, name, command))
continue;
DocumentClass const * c = it->second;
DocumentClassConstPtr c = it->second;
Layout const * layout = findLayoutWithoutModule(*c, name, command);
InsetLayout const * insetlayout = layout ? 0 :
findInsetLayoutWithoutModule(*c, name, command);