From db58111360d68f76c5797069a7b80967447d1379 Mon Sep 17 00:00:00 2001 From: Guillaume MM Date: Sun, 28 May 2017 13:25:53 +0200 Subject: [PATCH] Properly track the lifetime of signals2::slots (#8261) Starting at 61b2bd5e, boost::bind was progressively replaced with std::bind. They are not interchangeable though. boost::bind implements the tracking of boost::signals{,2}::trackable objects. Now that std::bind has completely replaced boost::bind, tracking never occurred. This commit replaces boost::signals2::trackable with the new preferred boost::signals2 methods: scoped_connections or slot::track_foreign. The support::Trackable class introduced is less safe but easier for transitioning old code. Fixes the crash at #8261. --- src/Converter.cpp | 22 ++++---------- src/LaTeX.h | 5 ++- src/Server.cpp | 11 ++++--- src/Server.h | 9 ++++-- src/frontends/qt4/GuiView.cpp | 3 +- src/frontends/qt4/GuiWorkArea.cpp | 7 +++-- src/graphics/GraphicsCacheItem.cpp | 25 +++++++-------- src/graphics/GraphicsCacheItem.h | 6 ++-- src/graphics/GraphicsConverter.cpp | 26 +++++++++------- src/graphics/GraphicsConverter.h | 8 +++-- src/graphics/GraphicsLoader.cpp | 22 ++++++++------ src/graphics/GraphicsLoader.h | 10 +++--- src/graphics/PreviewImage.cpp | 12 +++----- src/graphics/PreviewLoader.cpp | 21 +++++++------ src/graphics/PreviewLoader.h | 9 +++--- src/insets/InsetExternal.cpp | 3 +- src/insets/InsetExternal.h | 4 +-- src/insets/RenderPreview.cpp | 23 +++++--------- src/insets/RenderPreview.h | 17 +++++------ src/support/FileMonitor.cpp | 3 +- src/support/FileMonitor.h | 8 ++--- src/support/ForkedCalls.cpp | 19 ++++++------ src/support/ForkedCalls.h | 20 ++++++------ src/support/Makefile.am | 1 + src/support/Timeout.h | 4 +-- src/support/signals.h | 49 ++++++++++++++++++++++++++++++ 26 files changed, 195 insertions(+), 152 deletions(-) create mode 100644 src/support/signals.h diff --git a/src/Converter.cpp b/src/Converter.cpp index 104ad0a42c..6e10b18704 100644 --- a/src/Converter.cpp +++ b/src/Converter.cpp @@ -693,20 +693,6 @@ bool Converters::scanLog(Buffer const & buffer, string const & /*command*/, } -namespace { - -class ShowMessage - : public boost::signals2::trackable { -public: - ShowMessage(Buffer const & b) : buffer_(b) {} - void operator()(docstring const & msg) const { buffer_.message(msg); } -private: - Buffer const & buffer_; -}; - -} - - bool Converters::runLaTeX(Buffer const & buffer, string const & command, OutputParams const & runparams, ErrorList & errorList) { @@ -719,8 +705,12 @@ bool Converters::runLaTeX(Buffer const & buffer, string const & command, buffer.filePath(), buffer.layoutPos(), buffer.lastPreviewError()); TeXErrors terr; - ShowMessage show(buffer); - latex.message.connect(show); + // The connection closes itself at the end of the scope when latex is + // destroyed. One cannot close (and destroy) buffer while the converter is + // running. + latex.message.connect([&buffer](docstring const & msg){ + buffer.message(msg); + }); int const result = latex.run(terr); if (result & LaTeX::ERRORS) diff --git a/src/LaTeX.h b/src/LaTeX.h index f5d66a52b8..44920c362a 100644 --- a/src/LaTeX.h +++ b/src/LaTeX.h @@ -18,8 +18,7 @@ #include "support/docstring.h" #include "support/FileName.h" - -#include +#include "support/signals.h" #include #include @@ -148,7 +147,7 @@ public: }; /// This signal emits an informative message - boost::signals2::signal message; + signals2::signal message; /** diff --git a/src/Server.cpp b/src/Server.cpp index b89e834f20..98e1d6632c 100644 --- a/src/Server.cpp +++ b/src/Server.cpp @@ -55,8 +55,7 @@ #include "support/lassert.h" #include "support/lstrings.h" #include "support/os.h" - -#include "support/bind.h" +#include "support/signals.h" #include @@ -859,8 +858,12 @@ int LyXComm::startPipe(string const & file, bool write) } if (!write) { - theApp()->registerSocketCallback(fd, - bind(&LyXComm::read_ready, this)); + // Make sure not to call read_ready after destruction. + weak_ptr tracker = tracker_.p(); + theApp()->registerSocketCallback(fd, [=](){ + if (!tracker.expired()) + read_ready(); + }); } return fd; diff --git a/src/Server.h b/src/Server.h index 1a46c89281..40021daa41 100644 --- a/src/Server.h +++ b/src/Server.h @@ -14,7 +14,7 @@ #ifndef SERVER_H #define SERVER_H -#include +#include "support/signals.h" #include @@ -30,7 +30,7 @@ namespace lyx { class Server; -/** This class managed the pipes used for communicating with clients. +/** This class manages the pipes used for communicating with clients. Usage: Initialize with pipe-filename-base, client class to receive messages, and callback-function that will be called with the messages. When you want to send, use "send()". @@ -38,7 +38,7 @@ class Server; a clean string interface. */ #ifndef _WIN32 -class LyXComm : public boost::signals2::trackable { +class LyXComm { #else class LyXComm : public QObject { Q_OBJECT @@ -189,6 +189,9 @@ private: /// Did we defer loading of files to another instance? bool deferred_loading_; + + /// Track object's liveness + support::Trackable tracker_; }; diff --git a/src/frontends/qt4/GuiView.cpp b/src/frontends/qt4/GuiView.cpp index f859338053..34e342a48b 100644 --- a/src/frontends/qt4/GuiView.cpp +++ b/src/frontends/qt4/GuiView.cpp @@ -529,7 +529,8 @@ GuiView::GuiView(int id) // Start autosave timer if (lyxrc.autosave) { - d.autosave_timeout_.timeout.connect(bind(&GuiView::autoSave, this)); + // The connection is closed when this is destroyed. + d.autosave_timeout_.timeout.connect([this](){ autoSave();}); d.autosave_timeout_.setTimeout(lyxrc.autosave * 1000); d.autosave_timeout_.start(); } diff --git a/src/frontends/qt4/GuiWorkArea.cpp b/src/frontends/qt4/GuiWorkArea.cpp index 95d1081844..5c174bf7b3 100644 --- a/src/frontends/qt4/GuiWorkArea.cpp +++ b/src/frontends/qt4/GuiWorkArea.cpp @@ -320,9 +320,10 @@ void GuiWorkArea::init() d->setCursorShape(Qt::IBeamCursor); - d->synthetic_mouse_event_.timeout.timeout.connect( - bind(&GuiWorkArea::generateSyntheticMouseEvent, - this)); + // This connection is closed at the same time as this is destroyed. + d->synthetic_mouse_event_.timeout.timeout.connect([this](){ + generateSyntheticMouseEvent(); + }); // Initialize the vertical Scroll Bar QObject::connect(verticalScrollBar(), SIGNAL(valueChanged(int)), diff --git a/src/graphics/GraphicsCacheItem.cpp b/src/graphics/GraphicsCacheItem.cpp index 306dcdbc3d..576ad92bc7 100644 --- a/src/graphics/GraphicsCacheItem.cpp +++ b/src/graphics/GraphicsCacheItem.cpp @@ -29,7 +29,6 @@ #include "support/lassert.h" #include "support/unique_ptr.h" -#include "support/bind.h" #include "support/TempFile.h" using namespace std; @@ -39,7 +38,7 @@ namespace lyx { namespace graphics { -class CacheItem::Impl : public boost::signals2::trackable { +class CacheItem::Impl { public: /// @@ -50,7 +49,7 @@ public: /** * If no file conversion is needed, then tryDisplayFormat() calls * loadImage() directly. - * \return true if a conversion is necessary and no error occurred. + * \return true if a conversion is necessary and no error occurred. */ bool tryDisplayFormat(FileName & filename, string & from); @@ -120,10 +119,7 @@ public: ImageStatus status_; /// This signal is emitted when the image loading status changes. - boost::signals2::signal statusChanged; - - /// The connection of the signal ConvProcess::finishedConversion, - boost::signals2::connection cc_; + signals2::signal statusChanged; /// unique_ptr converter_; @@ -199,7 +195,7 @@ ImageStatus CacheItem::status() const } -boost::signals2::connection CacheItem::connect(slot_type const & slot) const +signals2::connection CacheItem::connect(slot_type const & slot) const { return pimpl_->statusChanged.connect(slot); } @@ -223,6 +219,7 @@ void CacheItem::Impl::startMonitor() if (monitor_) return; monitor_ = FileSystemWatcher::activeMonitor(filename_); + // Disconnected at the same time as this is destroyed. monitor_->connect([=](){ startLoading(); }); } @@ -254,9 +251,6 @@ void CacheItem::Impl::reset() status_ = WaitingToLoad; - if (cc_.connected()) - cc_.disconnect(); - if (converter_) converter_.reset(); } @@ -280,7 +274,6 @@ void CacheItem::Impl::imageConverted(bool success) file_to_load_ = converter_ ? FileName(converter_->convertedFile()) : FileName(); converter_.reset(); - cc_.disconnect(); success = !file_to_load_.empty() && file_to_load_.isReadableFile(); @@ -449,9 +442,13 @@ void CacheItem::Impl::convertToDisplayFormat() // Connect a signal to this->imageConverted and pass this signal to // the graphics converter so that we can load the modified file // on completion of the conversion process. - converter_ = make_unique(doc_file_, filename, to_file_base.absFileName(), + converter_ = make_unique(doc_file_, filename, + to_file_base.absFileName(), from, to_); - converter_->connect(bind(&Impl::imageConverted, this, _1)); + // Connection is closed at the same time as *this is destroyed. + converter_->connect([this](bool success){ + imageConverted(success); + }); converter_->startConversion(); } diff --git a/src/graphics/GraphicsCacheItem.h b/src/graphics/GraphicsCacheItem.h index 9cee2b872e..6f7f968bc6 100644 --- a/src/graphics/GraphicsCacheItem.h +++ b/src/graphics/GraphicsCacheItem.h @@ -30,7 +30,7 @@ #include "GraphicsTypes.h" -#include +#include "support/signals.h" namespace lyx { @@ -82,9 +82,9 @@ public: /** Connect and you'll be informed when the loading status of the image * changes. */ - typedef boost::signals2::signal::slot_type slot_type; + typedef signals2::signal::slot_type slot_type; /// - boost::signals2::connection connect(slot_type const &) const; + signals2::connection connect(slot_type const &) const; private: /// noncopyable diff --git a/src/graphics/GraphicsConverter.cpp b/src/graphics/GraphicsConverter.cpp index 63b6a44633..598e108b73 100644 --- a/src/graphics/GraphicsConverter.cpp +++ b/src/graphics/GraphicsConverter.cpp @@ -27,7 +27,6 @@ #include "support/lstrings.h" #include "support/os.h" -#include "support/bind.h" #include "support/TempFile.h" #include @@ -40,10 +39,12 @@ namespace lyx { namespace graphics { -class Converter::Impl : public boost::signals2::trackable { +class Converter::Impl { public: /// - Impl(FileName const &, FileName const &, string const &, string const &, string const &); + Impl(FileName const & doc_fname, + FileName const & from_file, string const & to_file_base, + string const & from_format, string const & to_format); /// void startConversion(); @@ -58,9 +59,9 @@ public: /** At the end of the conversion process inform the outside world * by emitting a signal. */ - typedef boost::signals2::signal SignalType; + typedef signals2::signal sig; /// - SignalType finishedConversion; + sig finishedConversion; /// FileName const doc_fname_; @@ -74,6 +75,8 @@ public: bool valid_process_; /// bool finished_; + /// + Trackable tracker_; }; @@ -85,8 +88,8 @@ bool Converter::isReachable(string const & from_format_name, Converter::Converter(FileName const & doc_fname, - FileName const & from_file, string const & to_file_base, - string const & from_format, string const & to_format) + FileName const & from_file, string const & to_file_base, + string const & from_format, string const & to_format) : pimpl_(new Impl(doc_fname, from_file, to_file_base, from_format, to_format)) {} @@ -103,7 +106,7 @@ void Converter::startConversion() const } -boost::signals2::connection Converter::connect(slot_type const & slot) const +signals2::connection Converter::connect(slot_type const & slot) const { return pimpl_->finishedConversion.connect(slot); } @@ -188,9 +191,10 @@ void Converter::Impl::startConversion() return; } - ForkedCall::SignalTypePtr ptr = - ForkedCallQueue::add(script_command_); - ptr->connect(bind(&Impl::converted, this, _1, _2)); + ForkedCall::sigPtr ptr = ForkedCallQueue::add(script_command_); + ptr->connect(ForkedCall::slot([this](pid_t pid, int retval){ + converted(pid, retval); + }).track_foreign(tracker_.p())); } diff --git a/src/graphics/GraphicsConverter.h b/src/graphics/GraphicsConverter.h index d3d0b8155d..c038029bd8 100644 --- a/src/graphics/GraphicsConverter.h +++ b/src/graphics/GraphicsConverter.h @@ -17,7 +17,8 @@ #ifndef GRAPHICSCONVERTER_H #define GRAPHICSCONVERTER_H -#include +#include "support/signals.h" + namespace lyx { @@ -47,11 +48,12 @@ public: /** Connect and you'll be informed when the conversion process has * finished. * If the conversion is successful, then the listener is passed \c true. + * The connection is closed when this is destroyed. */ - typedef boost::signals2::signal sig_type; + typedef signals2::signal sig_type; typedef sig_type::slot_type slot_type; /// - boost::signals2::connection connect(slot_type const &) const; + signals2::connection connect(slot_type const &) const; /** If the conversion is successful, this returns the name of the * resulting file. diff --git a/src/graphics/GraphicsLoader.cpp b/src/graphics/GraphicsLoader.cpp index b9c1e76c6e..15f70b84e8 100644 --- a/src/graphics/GraphicsLoader.cpp +++ b/src/graphics/GraphicsLoader.cpp @@ -107,16 +107,17 @@ void LoaderQueue::loadNext() LoaderQueue::LoaderQueue() : timer(s_millisecs_, Timeout::ONETIME), - running_(false) + running_(false) { - timer.timeout.connect(bind(&LoaderQueue::loadNext, this)); + // Disconnected when this is destroyed + timer.timeout.connect([this](){ loadNext(); }); } void LoaderQueue::startLoader() { LYXERR(Debug::GRAPHICS, "LoaderQueue: waking up"); - running_ = true ; + running_ = true; timer.setTimeout(s_millisecs_); timer.start(); } @@ -163,7 +164,7 @@ void LoaderQueue::touch(Cache::ItemPtr const & item) typedef std::shared_ptr ImagePtr; -class Loader::Impl : public boost::signals2::trackable { +class Loader::Impl { friend class Loader; public: /// @@ -192,9 +193,9 @@ public: /// We modify a local copy of the image once it is loaded. ImagePtr image_; /// This signal is emitted when the image loading status changes. - boost::signals2::signal signal_; - /// The connection of the signal StatusChanged - boost::signals2::connection sc_; + signals2::signal signal_; + /// The connection of the signal statusChanged + signals2::scoped_connection connection_; double displayPixelRatio() const { @@ -363,7 +364,7 @@ void Loader::setDisplayPixelRatio(double scale) } -boost::signals2::connection Loader::connect(slot_type const & slot) const +signals2::connection Loader::connect(slot const & slot) const { return pimpl_->signal_.connect(slot); } @@ -405,7 +406,7 @@ void Loader::Impl::resetFile(FileName const & file) // signal needs to be disconnected. try { // This can in theory throw a BufferException - sc_.disconnect(); + connection_.disconnect(); } catch (...) { LYXERR(Debug::GRAPHICS, "Unable to disconnect signal."); } @@ -434,7 +435,8 @@ void Loader::Impl::resetFile(FileName const & file) if (continue_monitoring && !cached_item_->monitoring()) cached_item_->startMonitoring(); - sc_ = cached_item_->connect(bind(&Impl::statusChanged, this)); + // This is a scoped connection + connection_ = cached_item_->connect([this](){ statusChanged(); }); } diff --git a/src/graphics/GraphicsLoader.h b/src/graphics/GraphicsLoader.h index 62ea303b34..0a299cba1a 100644 --- a/src/graphics/GraphicsLoader.h +++ b/src/graphics/GraphicsLoader.h @@ -26,7 +26,7 @@ #include "GraphicsTypes.h" -#include +#include "support/signals.h" namespace lyx { @@ -70,7 +70,7 @@ public: */ void startLoading() const; - /** Tries to reload the image. + /** Tries to reload the image. */ void reload() const; @@ -90,10 +90,10 @@ public: /** Connect and you'll be informed when the loading status of the image * changes. */ - typedef boost::signals2::signal sig_type; - typedef sig_type::slot_type slot_type; + typedef signals2::signal sig; + typedef sig::slot_type slot; /// - boost::signals2::connection connect(slot_type const &) const; + signals2::connection connect(slot const &) const; /** The loaded image with Pixmap set. * If the Pixmap is not yet set (see status() for why...), returns 0. diff --git a/src/graphics/PreviewImage.cpp b/src/graphics/PreviewImage.cpp index da0f1e21ce..b80bf9415b 100644 --- a/src/graphics/PreviewImage.cpp +++ b/src/graphics/PreviewImage.cpp @@ -20,7 +20,6 @@ #include "support/FileName.h" -#include "support/bind.h" using namespace std; using namespace lyx::support; @@ -28,7 +27,7 @@ using namespace lyx::support; namespace lyx { namespace graphics { -class PreviewImage::Impl : public boost::signals2::trackable { +class PreviewImage::Impl { public: /// Impl(PreviewImage & p, PreviewLoader & l, @@ -105,15 +104,14 @@ PreviewLoader & PreviewImage::previewLoader() const } -PreviewImage::Impl::Impl(PreviewImage & p, PreviewLoader & l, - string const & s, - FileName const & bf, - double af) +PreviewImage::Impl::Impl(PreviewImage & p, PreviewLoader & l, string const & s, + FileName const & bf, double af) : parent_(p), ploader_(l), iloader_(l.buffer().fileName(), bf), snippet_(s), ascent_frac_(af) { iloader_.setDisplayPixelRatio(l.displayPixelRatio()); - iloader_.connect(bind(&Impl::statusChanged, this)); + // This connection is destroyed at the same time as this. + iloader_.connect([this](){ statusChanged(); }); } diff --git a/src/graphics/PreviewLoader.cpp b/src/graphics/PreviewLoader.cpp index 93ea6b79b6..49b8b73b0a 100644 --- a/src/graphics/PreviewLoader.cpp +++ b/src/graphics/PreviewLoader.cpp @@ -38,7 +38,6 @@ #include "support/ForkedCalls.h" #include "support/lstrings.h" -#include "support/bind.h" #include "support/TempFile.h" #include @@ -168,7 +167,7 @@ typedef InProgressProcesses::value_type InProgressProcess; namespace lyx { namespace graphics { -class PreviewLoader::Impl : public boost::signals2::trackable { +class PreviewLoader::Impl { public: /// Impl(PreviewLoader & p, Buffer const & b); @@ -189,7 +188,7 @@ public: void refreshPreviews(); /// Emit this signal when an image is ready for display. - boost::signals2::signal imageReady; + signals2::signal imageReady; Buffer const & buffer() const { return buffer_; } @@ -240,6 +239,8 @@ private: /// We don't own this static lyx::Converter const * pconverter_; + + signals2::scoped_connection connection_; }; @@ -297,7 +298,7 @@ void PreviewLoader::refreshPreviews() } -boost::signals2::connection PreviewLoader::connect(slot_type const & slot) const +signals2::connection PreviewLoader::connect(slot const & slot) const { return pimpl_->imageReady.connect(slot); } @@ -708,12 +709,12 @@ void PreviewLoader::Impl::startLoading(bool wait) << " " << quoteName(latexfile.toFilesystemEncoding()) << " --dpi " << font_scaling_factor_; - // FIXME XHTML + // FIXME XHTML // The colors should be customizable. if (!buffer_.isExporting()) { ColorCode const fg = PreviewLoader::foregroundColor(); ColorCode const bg = PreviewLoader::backgroundColor(); - cs << " --fg " << theApp()->hexName(fg) + cs << " --fg " << theApp()->hexName(fg) << " --bg " << theApp()->hexName(bg); } @@ -737,9 +738,11 @@ void PreviewLoader::Impl::startLoading(bool wait) } // Initiate the conversion from LaTeX to bitmap images files. - ForkedCall::SignalTypePtr - convert_ptr(new ForkedCall::SignalType); - convert_ptr->connect(bind(&Impl::finishedGenerating, this, _1, _2)); + ForkedCall::sigPtr convert_ptr = make_shared(); + // This is a scoped connection + connection_ = convert_ptr->connect([this](pid_t pid, int retval){ + finishedGenerating(pid, retval); + }); ForkedCall call(buffer_.filePath()); int ret = call.startScript(command, convert_ptr); diff --git a/src/graphics/PreviewLoader.h b/src/graphics/PreviewLoader.h index 3239ffcf17..ca22a9fa5c 100644 --- a/src/graphics/PreviewLoader.h +++ b/src/graphics/PreviewLoader.h @@ -18,7 +18,8 @@ #ifndef PREVIEWLOADER_H #define PREVIEWLOADER_H -#include +#include "support/signals.h" + #include #include "ColorCode.h" @@ -76,10 +77,10 @@ public: * has been created and is ready for loading through * lyx::graphics::PreviewImage::image(). */ - typedef boost::signals2::signal sig_type; - typedef sig_type::slot_type slot_type; + typedef signals2::signal sig; + typedef sig::slot_type slot; /// - boost::signals2::connection connect(slot_type const &) const; + signals2::connection connect(slot const &) const; /** When PreviewImage has finished loading the image file into memory, * it tells the PreviewLoader to tell the outside world diff --git a/src/insets/InsetExternal.cpp b/src/insets/InsetExternal.cpp index 8efdd3e97e..3b8159bdb3 100644 --- a/src/insets/InsetExternal.cpp +++ b/src/insets/InsetExternal.cpp @@ -39,7 +39,6 @@ #include "graphics/PreviewLoader.h" -#include "support/bind.h" #include "support/convert.h" #include "support/debug.h" #include "support/ExceptionMessage.h" @@ -428,7 +427,6 @@ InsetExternal::InsetExternal(Buffer * buf) // Mouse hover is not copied and remains empty InsetExternal::InsetExternal(InsetExternal const & other) : Inset(other), - boost::signals2::trackable(), params_(other.params_), renderer_(other.renderer_->clone(this)) {} @@ -635,6 +633,7 @@ void InsetExternal::setParams(InsetExternalParams const & p) case PREVIEW_INSTANT: { renderer_ = make_unique(this); RenderMonitoredPreview * preview_ptr = renderer_->asMonitoredPreview(); + // This connection is closed at the same time as this is destroyed. preview_ptr->connect([=]() { fileChanged(); }); add_preview_and_start_loading(*preview_ptr, *this, buffer()); break; diff --git a/src/insets/InsetExternal.h b/src/insets/InsetExternal.h index 5eca6641cc..420550f924 100644 --- a/src/insets/InsetExternal.h +++ b/src/insets/InsetExternal.h @@ -19,8 +19,6 @@ #include "support/FileName.h" #include "support/unique_ptr.h" -#include - namespace lyx { @@ -90,7 +88,7 @@ private: class RenderBase; /// -class InsetExternal : public Inset, public boost::signals2::trackable +class InsetExternal : public Inset { // Disable assignment operator, since it is not used, and it is too // complicated to implement it consistently with the copy constructor diff --git a/src/insets/RenderPreview.cpp b/src/insets/RenderPreview.cpp index e29ccc4271..75e709d577 100644 --- a/src/insets/RenderPreview.cpp +++ b/src/insets/RenderPreview.cpp @@ -31,8 +31,6 @@ #include "support/lassert.h" #include "support/lstrings.h" -#include "support/bind.h" - using namespace std; using namespace lyx::support; @@ -77,19 +75,11 @@ RenderPreview::RenderPreview(Inset const * inset) RenderPreview::RenderPreview(RenderPreview const & other, Inset const * inset) : RenderBase(other), - boost::signals2::trackable(), snippet_(other.snippet_), parent_(inset) {} -RenderPreview::~RenderPreview() -{ - if (ploader_connection_.connected()) - ploader_connection_.disconnect(); -} - - RenderBase * RenderPreview::clone(Inset const * inset) const { return new RenderPreview(*this, inset); @@ -241,10 +231,12 @@ void RenderPreview::addPreview(docstring const & latex_snippet, // If this is the first time of calling, connect to the // PreviewLoader signal that'll inform us when the preview image // is ready for loading. - if (!ploader_connection_.connected()) { - ploader_connection_ = ploader.connect( - bind(&RenderPreview::imageReady, this, _1)); - } + if (!ploader_connection_.connected()) + // This is a scoped connection. + ploader_connection_ = + ploader.connect([this](graphics::PreviewImage const & pi){ + imageReady(pi); + }); ploader.add(snippet_); } @@ -296,8 +288,7 @@ void RenderMonitoredPreview::draw(PainterInfo & pi, int x, int y) const } -boost::signals2::connection -RenderMonitoredPreview::connect(ChangedSig::slot_type const & slot) +signals2::connection RenderMonitoredPreview::connect(slot const & slot) { return changed_.connect(slot); } diff --git a/src/insets/RenderPreview.h b/src/insets/RenderPreview.h index 42d944d035..2f83aff5dc 100644 --- a/src/insets/RenderPreview.h +++ b/src/insets/RenderPreview.h @@ -21,10 +21,8 @@ #include "support/docstring.h" #include "support/FileMonitor.h" #include "support/FileName.h" +#include "support/signals.h" -#include -#include -#include namespace lyx { @@ -40,7 +38,7 @@ class PreviewLoader; } // namespace graphics -class RenderPreview : public RenderBase, public boost::signals2::trackable { +class RenderPreview : public RenderBase { public: /// Return true if preview is enabled in text (from LyXRC::preview) static bool previewText(); @@ -49,7 +47,6 @@ public: RenderPreview(Inset const *); RenderPreview(RenderPreview const &, Inset const *); - ~RenderPreview(); RenderBase * clone(Inset const *) const; /// Compute the size of the object, returned in dim @@ -104,7 +101,7 @@ private: /** Store the connection to the preview loader so that we connect * only once. */ - boost::signals2::connection ploader_connection_; + signals2::scoped_connection ploader_connection_; /// Inform the core that the inset has changed. Inset const * parent_; @@ -124,15 +121,17 @@ public: void stopMonitoring() const; /// Connect and you'll be informed when the file changes. - typedef boost::signals2::signal ChangedSig; - boost::signals2::connection connect(ChangedSig::slot_type const &); + /// Do not forget to track objects used by the slot. + typedef signals2::signal sig; + typedef sig::slot_type slot; + signals2::connection connect(slot const & slot); /// equivalent to dynamic_cast virtual RenderMonitoredPreview * asMonitoredPreview() { return this; } private: /// This signal is emitted if the file is modified - ChangedSig changed_; + sig changed_; /// mutable support::ActiveFileMonitorPtr monitor_; /// diff --git a/src/support/FileMonitor.cpp b/src/support/FileMonitor.cpp index 30a4170ed5..f1fefb8d18 100644 --- a/src/support/FileMonitor.cpp +++ b/src/support/FileMonitor.cpp @@ -176,8 +176,7 @@ void FileMonitor::reconnectToFileMonitorGuard() } -boost::signals2::connection -FileMonitor::connect(sig::slot_type const & slot) +signals2::connection FileMonitor::connect(slot const & slot) { return fileChanged_.connect(slot); } diff --git a/src/support/FileMonitor.h b/src/support/FileMonitor.h index 23302ed7b1..49f12cba72 100644 --- a/src/support/FileMonitor.h +++ b/src/support/FileMonitor.h @@ -17,6 +17,7 @@ #define FILEMONITOR_H #include "support/FileName.h" +#include "support/signals.h" #include @@ -24,8 +25,6 @@ #include #include -#include - namespace lyx { namespace support { @@ -158,9 +157,10 @@ class FileMonitor : public QObject public: FileMonitor(std::shared_ptr monitor); - typedef boost::signals2::signal sig; + typedef signals2::signal sig; + typedef sig::slot_type slot; /// Connect and you'll be informed when the file has changed. - boost::signals2::connection connect(sig::slot_type const &); + signals2::connection connect(slot const &); /// disconnect all slots connected to the boost signal fileChanged_ or to /// the qt signal fileChanged() void disconnect(); diff --git a/src/support/ForkedCalls.cpp b/src/support/ForkedCalls.cpp index 4827947d37..56f7bc9d71 100644 --- a/src/support/ForkedCalls.cpp +++ b/src/support/ForkedCalls.cpp @@ -58,7 +58,7 @@ namespace { // ///////////////////////////////////////////////////////////////////// -class Murder : public boost::signals2::trackable { +class Murder { public: // static void killItDead(int secs, pid_t pid) @@ -83,7 +83,8 @@ private: Murder(int secs, pid_t pid) : timeout_(1000*secs, Timeout::ONETIME), pid_(pid) { - timeout_.timeout.connect(lyx::bind(&Murder::kill, this)); + // Connection is closed with this. + timeout_.timeout.connect([this](){ kill(); }); timeout_.start(); } @@ -277,7 +278,7 @@ ForkedCall::ForkedCall(string const & path, string const & lpath) int ForkedCall::startScript(Starttype wait, string const & what) { if (wait != Wait) { - retval_ = startScript(what, SignalTypePtr()); + retval_ = startScript(what, sigPtr()); return retval_; } @@ -287,7 +288,7 @@ int ForkedCall::startScript(Starttype wait, string const & what) } -int ForkedCall::startScript(string const & what, SignalTypePtr signal) +int ForkedCall::startScript(string const & what, sigPtr signal) { command_ = commandPrep(trim(what)); signal_ = signal; @@ -435,13 +436,13 @@ int ForkedCall::generateChild() namespace ForkedCallQueue { /// A process in the queue -typedef pair Process; +typedef pair Process; /** Add a process to the queue. Processes are forked sequentially * only one is running at a time. * Connect to the returned signal and you'll be informed when * the process has ended. */ -ForkedCall::SignalTypePtr add(string const & process); +ForkedCall::sigPtr add(string const & process); /// in-progress queue static queue callQueue_; @@ -456,10 +457,10 @@ void stopCaller(); /// void callback(pid_t, int); -ForkedCall::SignalTypePtr add(string const & process) +ForkedCall::sigPtr add(string const & process) { - ForkedCall::SignalTypePtr ptr; - ptr.reset(new ForkedCall::SignalType); + ForkedCall::sigPtr ptr; + ptr.reset(new ForkedCall::sig); callQueue_.push(Process(process, ptr)); if (!running_) startCaller(); diff --git a/src/support/ForkedCalls.h b/src/support/ForkedCalls.h index 1ed2f75ad5..452c9792c4 100644 --- a/src/support/ForkedCalls.h +++ b/src/support/ForkedCalls.h @@ -14,8 +14,8 @@ #ifndef FORKEDCALLS_H #define FORKEDCALLS_H +#include "support/signals.h" #include "support/strfwd.h" -#include #ifdef HAVE_SYS_TYPES_H # include @@ -44,7 +44,7 @@ public: /// virtual std::shared_ptr clone() const = 0; - /** A SignalType signal can be emitted once the forked process + /** A Signal signal can be emitted once the forked process * has finished. It passes: * the PID of the child and; * the return value from the child. @@ -53,7 +53,8 @@ public: * we can return easily to C++ methods, rather than just globally * accessible functions. */ - typedef boost::signals2::signal SignalType; + typedef signals2::signal sig; + typedef sig::slot_type slot; /** The signal is connected in the calling routine to the desired * slot. We pass a shared_ptr rather than a reference to the signal @@ -61,9 +62,10 @@ public: * class (and hence the signal) to be destructed before the forked * call is complete. * - * It doesn't matter if the slot disappears, SigC takes care of that. + * Use Slot::track or Signal::scoped_connection to ensure that the + * connection is closed before the slot expires. */ - typedef std::shared_ptr SignalTypePtr; + typedef std::shared_ptr sigPtr; /** Invoking the following methods makes sense only if the command * is running asynchronously! @@ -114,7 +116,7 @@ protected: pid_t fork(); /// Callback function - SignalTypePtr signal_; + sigPtr signal_; /// identifying command (for display in the GUI perhaps). std::string command_; @@ -136,7 +138,7 @@ private: }; -/** +/** * An instance of class ForkedCall represents a single child process. * * Class ForkedCall uses fork() and execvp() to lauch the child process. @@ -175,7 +177,7 @@ public: int startScript(Starttype, std::string const & what); /// - int startScript(std::string const & what, SignalTypePtr); + int startScript(std::string const & what, sigPtr ptr); private: /// @@ -195,7 +197,7 @@ private: namespace ForkedCallQueue { -ForkedCall::SignalTypePtr add(std::string const & process); +ForkedCall::sigPtr add(std::string const & process); /// Query whether the queue is running a forked process now. bool running(); diff --git a/src/support/Makefile.am b/src/support/Makefile.am index 9865d6fe2d..d3c902cc8e 100644 --- a/src/support/Makefile.am +++ b/src/support/Makefile.am @@ -93,6 +93,7 @@ liblyxsupport_a_SOURCES = \ qstring_helpers.h \ regex.h \ RefChanger.h \ + signals.h \ socktools.cpp \ socktools.h \ strfwd.h \ diff --git a/src/support/Timeout.h b/src/support/Timeout.h index 042ed4587b..eef78dbec6 100644 --- a/src/support/Timeout.h +++ b/src/support/Timeout.h @@ -12,7 +12,7 @@ #ifndef TIMEOUT_H #define TIMEOUT_H -#include +#include "support/signals.h" namespace lyx { @@ -40,7 +40,7 @@ public: /// restart the timer void restart(); /// signal emitted on timer expiry - boost::signals2::signal timeout; + signals2::signal timeout; /// emit the signal void emit(); /// set the timer type diff --git a/src/support/signals.h b/src/support/signals.h new file mode 100644 index 0000000000..7d4d1161e5 --- /dev/null +++ b/src/support/signals.h @@ -0,0 +1,49 @@ +// -*- C++ -*- +/** + * \file signals.h + * This file is part of LyX, the document processor. + * Licence details can be found in the file COPYING. + * + * \author Guillaume Munch + * + * Full author contact details are available in file CREDITS. + */ + +#ifndef LYX_SIGNALS_H +#define LYX_SIGNALS_H + +#include "boost/signals2.hpp" + +#include + +namespace lyx { + +namespace signals2 = ::boost::signals2; + +namespace support { + +/// A small utility to use with signals2::slot_type::track_foreign when the +/// parent object is not handled by a shared_ptr, or to track the lifetime of an +/// object. Using Trackable to track lifetimes is less thread-safe than tracking +/// their parents directly with a shared_ptr as recommended by signals2, but it +/// makes it easier for transitioning old code. (Essentially because Trackable +/// will not prevent the deletion of the parent by a concurrent thread.) +class Trackable { +public: + Trackable() : p_(std::make_shared(0)) {} + Trackable(Trackable const &) : Trackable() {} + Trackable(Trackable &&) : Trackable() {} + Trackable & operator=(Trackable const &) { return *this; } + Trackable & operator=(Trackable &&) { return *this; } + // This weak pointer lets you know if the parent object has been destroyed + std::weak_ptr p() const { return p_; } +private: + std::shared_ptr const p_; +}; + +} // namespace support + +} // namespace lyx + + +#endif