diff --git a/src/app/CMakeLists.txt b/src/app/CMakeLists.txt index 0d7fdc29a..f2b69aba2 100644 --- a/src/app/CMakeLists.txt +++ b/src/app/CMakeLists.txt @@ -331,6 +331,7 @@ add_library(app-lib res/resources_loader.cpp resource_finder.cpp restore_visible_layers.cpp + rw_lock.cpp script/app_object.cpp script/app_scripting.cpp script/console_object.cpp diff --git a/src/app/context_flags.cpp b/src/app/context_flags.cpp index c054a5ba6..7f3c0943c 100644 --- a/src/app/context_flags.cpp +++ b/src/app/context_flags.cpp @@ -44,7 +44,7 @@ void ContextFlags::update(Context* context) updateFlagsFromSite(site); - if (document->lockToWrite(0)) + if (document->upgradeToWrite(0)) m_flags |= ActiveDocumentIsWritable; document->unlock(); diff --git a/src/app/crash/backup_observer.cpp b/src/app/crash/backup_observer.cpp index c7e2036b2..4334fc2bd 100644 --- a/src/app/crash/backup_observer.cpp +++ b/src/app/crash/backup_observer.cpp @@ -104,8 +104,12 @@ void BackupObserver::backgroundThread() for (app::Document* doc : m_documents) { try { - if (doc->needsBackup()) - m_session->saveDocumentChanges(doc); + if (doc->needsBackup()) { + if (!m_session->saveDocumentChanges(doc)) { + TRACE("RECO: Document '%d' backup was canceled by UI\n", doc->id()); + somethingLocked = true; + } + } } catch (const std::exception&) { TRACE("RECO: Document '%d' is locked\n", doc->id()); diff --git a/src/app/crash/session.cpp b/src/app/crash/session.cpp index e2ce29def..5f372f33b 100644 --- a/src/app/crash/session.cpp +++ b/src/app/crash/session.cpp @@ -26,6 +26,7 @@ #include "base/split_string.h" #include "base/string.h" #include "base/unique_ptr.h" +#include "doc/cancel_io.h" namespace app { namespace crash { @@ -141,9 +142,25 @@ void Session::removeFromDisk() } } -void Session::saveDocumentChanges(app::Document* doc) +class CustomWeakDocumentReader : public WeakDocumentReader + , public doc::CancelIO { +public: + explicit CustomWeakDocumentReader(Document* doc) + : WeakDocumentReader(doc) { + } + + // CancelIO impl + bool isCanceled() override { + return !isLocked(); + } +}; + +bool Session::saveDocumentChanges(app::Document* doc) { - DocumentReader reader(doc, 250); + CustomWeakDocumentReader reader(doc); + if (!reader.isLocked()) + return false; + app::Context ctx; std::string dir = base::join_path(m_path, base::convert_to(doc->id())); @@ -153,7 +170,7 @@ void Session::saveDocumentChanges(app::Document* doc) base::make_directory(dir); // Save document information - write_document(dir, doc); + return write_document(dir, doc, &reader); } void Session::removeDocument(app::Document* doc) diff --git a/src/app/crash/session.h b/src/app/crash/session.h index d281c626f..2016a5800 100644 --- a/src/app/crash/session.h +++ b/src/app/crash/session.h @@ -50,7 +50,7 @@ namespace crash { void create(base::pid pid); void removeFromDisk(); - void saveDocumentChanges(app::Document* doc); + bool saveDocumentChanges(app::Document* doc); void removeDocument(app::Document* doc); void restoreBackup(Backup* backup); diff --git a/src/app/crash/write_document.cpp b/src/app/crash/write_document.cpp index 52e7210a6..723d6df80 100644 --- a/src/app/crash/write_document.cpp +++ b/src/app/crash/write_document.cpp @@ -18,6 +18,7 @@ #include "base/serialization.h" #include "base/string.h" #include "base/unique_ptr.h" +#include "doc/cancel_io.h" #include "doc/cel.h" #include "doc/cel_data_io.h" #include "doc/cel_io.h" @@ -45,52 +46,74 @@ using namespace doc; namespace { static std::map g_docVersions; +static std::map > g_deleteFiles; class Writer { public: - Writer(const std::string& dir, app::Document* doc) + Writer(const std::string& dir, app::Document* doc, doc::CancelIO* cancel) : m_dir(dir) , m_doc(doc) - , m_objVersions(g_docVersions[doc->id()]) { + , m_objVersions(g_docVersions[doc->id()]) + , m_deleteFiles(g_deleteFiles[doc->id()]) + , m_cancel(cancel) { } - void saveDocument() { + bool saveDocument() { Sprite* spr = m_doc->sprite(); // Save from objects without children (e.g. images), to aggregated // objects (e.g. cels, layers, etc.) for (Palette* pal : spr->getPalettes()) - saveObject("pal", pal, &Writer::writePalette); + if (!saveObject("pal", pal, &Writer::writePalette)) + return false; for (FrameTag* frtag : spr->frameTags()) - saveObject("frtag", frtag, &Writer::writeFrameTag); + if (!saveObject("frtag", frtag, &Writer::writeFrameTag)) + return false; for (Cel* cel : spr->uniqueCels()) { - saveObject("img", cel->image(), &Writer::writeImage); - saveObject("celdata", cel->data(), &Writer::writeCelData); + if (!saveObject("img", cel->image(), &Writer::writeImage)) + return false; + + if (!saveObject("celdata", cel->data(), &Writer::writeCelData)) + return false; } for (Cel* cel : spr->cels()) - saveObject("cel", cel, &Writer::writeCel); + if (!saveObject("cel", cel, &Writer::writeCel)) + return false; // Save all layers (top level, groups, children, etc.) LayerList layers = spr->allLayers(); for (Layer* lay : layers) - saveObject("lay", lay, &Writer::writeLayerStructure); + if (!saveObject("lay", lay, &Writer::writeLayerStructure)) + return false; - saveObject("spr", spr, &Writer::writeSprite); - saveObject("doc", m_doc, &Writer::writeDocumentFile); + if (!saveObject("spr", spr, &Writer::writeSprite)) + return false; + + if (!saveObject("doc", m_doc, &Writer::writeDocumentFile)) + return false; + + // Delete old files after all files are correctly saved. + deleteOldVersions(); + return true; } private: - void writeDocumentFile(std::ofstream& s, app::Document* doc) { - write32(s, doc->sprite()->id()); - write_string(s, doc->filename()); + bool isCanceled() const { + return (m_cancel && m_cancel->isCanceled()); } - void writeSprite(std::ofstream& s, Sprite* spr) { + bool writeDocumentFile(std::ofstream& s, app::Document* doc) { + write32(s, doc->sprite()->id()); + write_string(s, doc->filename()); + return true; + } + + bool writeSprite(std::ofstream& s, Sprite* spr) { write8(s, spr->pixelFormat()); write16(s, spr->width()); write16(s, spr->height()); @@ -114,6 +137,8 @@ private: write32(s, spr->frameTags().size()); for (FrameTag* frtag : spr->frameTags()) write32(s, frtag->id()); + + return true; } void writeAllLayersID(std::ofstream& s, ObjectId parentId, const LayerGroup* group) { @@ -126,7 +151,7 @@ private: } } - void writeLayerStructure(std::ofstream& s, Layer* lay) { + bool writeLayerStructure(std::ofstream& s, Layer* lay) { write32(s, static_cast(lay->flags())); // Flags write16(s, static_cast(lay->type())); // Type write_string(s, lay->name()); @@ -151,36 +176,44 @@ private: // writeSprite/writeAllLayersID() functions) break; } + return true; } - void writeCel(std::ofstream& s, Cel* cel) { + bool writeCel(std::ofstream& s, Cel* cel) { write_cel(s, cel); + return true; } - void writeCelData(std::ofstream& s, CelData* celdata) { + bool writeCelData(std::ofstream& s, CelData* celdata) { write_celdata(s, celdata); + return true; } - void writeImage(std::ofstream& s, Image* img) { - write_image(s, img); + bool writeImage(std::ofstream& s, Image* img) { + return write_image(s, img, m_cancel); } - void writePalette(std::ofstream& s, Palette* pal) { + bool writePalette(std::ofstream& s, Palette* pal) { write_palette(s, pal); + return true; } - void writeFrameTag(std::ofstream& s, FrameTag* frameTag) { + bool writeFrameTag(std::ofstream& s, FrameTag* frameTag) { write_frame_tag(s, frameTag); + return true; } template - void saveObject(const char* prefix, T* obj, void (Writer::*writeMember)(std::ofstream&, T*)) { + bool saveObject(const char* prefix, T* obj, bool (Writer::*writeMember)(std::ofstream&, T*)) { + if (isCanceled()) + return false; + if (!obj->version()) obj->incrementVersion(); ObjVersions& versions = m_objVersions[obj->id()]; if (versions.newer() == obj->version()) - return; + return true; std::string fn = prefix; fn.push_back('-'); @@ -192,7 +225,8 @@ private: std::ofstream s(FSTREAM_PATH(fullfn), std::ofstream::binary); write32(s, 0); // Leave a room for the magic number - (this->*writeMember)(s, obj); // Write the object + if (!(this->*writeMember)(s, obj)) // Write the object + return false; // Flush all data. In this way we ensure that the magic number is // the last thing being written in the file. @@ -203,23 +237,36 @@ private: write32(s, MAGIC_NUMBER); // Remove the older version - try { - if (versions.older() && base::is_file(oldfn)) - base::delete_file(oldfn); - } - catch (const std::exception&) { - TRACE(" - Cannot delete %s #%d v%d\n", prefix, obj->id(), versions.older()); - } + if (versions.older() && base::is_file(oldfn)) + m_deleteFiles.push_back(oldfn); // Rotate versions and add the latest one versions.rotateRevisions(obj->version()); TRACE(" - Saved %s #%d v%d\n", prefix, obj->id(), obj->version()); + return true; + } + + void deleteOldVersions() { + while (!m_deleteFiles.empty() && !isCanceled()) { + std::string file = m_deleteFiles.back(); + m_deleteFiles.erase(m_deleteFiles.end()-1); + + try { + TRACE(" - Deleting <%s>\n", file.c_str()); + base::delete_file(file); + } + catch (const std::exception&) { + TRACE(" - Cannot delete <%s>\n", file.c_str()); + } + } } std::string m_dir; app::Document* m_doc; ObjVersionsMap& m_objVersions; + std::vector& m_deleteFiles; + doc::CancelIO* m_cancel; }; } // anonymous namespace @@ -227,21 +274,30 @@ private: ////////////////////////////////////////////////////////////////////// // Public API -void write_document(const std::string& dir, app::Document* doc) +bool write_document(const std::string& dir, + app::Document* doc, + doc::CancelIO* cancel) { - Writer writer(dir, doc); - writer.saveDocument(); + Writer writer(dir, doc, cancel); + return writer.saveDocument(); } void delete_document_internals(app::Document* doc) { ASSERT(doc); - auto it = g_docVersions.find(doc->id()); // The document could not be inside g_documentObjects in case it was // never saved by the backup process. - if (it != g_docVersions.end()) - g_docVersions.erase(it); + { + auto it = g_docVersions.find(doc->id()); + if (it != g_docVersions.end()) + g_docVersions.erase(it); + } + { + auto it = g_deleteFiles.find(doc->id()); + if (it != g_deleteFiles.end()) + g_deleteFiles.erase(it); + } } } // namespace crash diff --git a/src/app/crash/write_document.h b/src/app/crash/write_document.h index 87c7fe651..d4a211992 100644 --- a/src/app/crash/write_document.h +++ b/src/app/crash/write_document.h @@ -1,5 +1,5 @@ // Aseprite -// Copyright (C) 2001-2015 David Capello +// Copyright (C) 2001-2016 David Capello // // This program is distributed under the terms of // the End-User License Agreement for Aseprite. @@ -10,14 +10,19 @@ #include +namespace doc { + class CancelIO; +} + namespace app { -class Document; -namespace crash { + class Document; - void write_document(const std::string& dir, app::Document* doc); - void delete_document_internals(app::Document* doc); + namespace crash { -} // namespace crash + bool write_document(const std::string& dir, app::Document* doc, doc::CancelIO* cancel); + void delete_document_internals(app::Document* doc); + + } // namespace crash } // namespace app #endif diff --git a/src/app/document.cpp b/src/app/document.cpp index a83dbc6c7..8112eadd3 100644 --- a/src/app/document.cpp +++ b/src/app/document.cpp @@ -8,10 +8,6 @@ #include "config.h" #endif -// Uncomment this line in case that you want TRACE() lock/unlock -// operations. -//#define DEBUG_DOCUMENT_LOCKS - #include "app/document.h" #include "app/app.h" @@ -25,9 +21,6 @@ #include "app/pref/preferences.h" #include "app/util/create_cel_copy.h" #include "base/memory.h" -#include "base/mutex.h" -#include "base/scoped_lock.h" -#include "base/thread.h" #include "base/unique_ptr.h" #include "doc/cel.h" #include "doc/context.h" @@ -50,8 +43,6 @@ using namespace doc; Document::Document(Sprite* sprite) : m_undo(new DocumentUndo) , m_associated_to_file(false) - , m_write_lock(false) - , m_read_locks(0) // Information about the file format used to load/save this document , m_format_options(NULL) // Mask @@ -436,130 +427,6 @@ Document* Document::duplicate(DuplicateType type) const return documentCopy.release(); } -////////////////////////////////////////////////////////////////////// -// Multi-threading ("sprite wrappers" use this) - -bool Document::lock(LockType lockType, int timeout) -{ - while (timeout >= 0) { - { - scoped_lock lock(m_mutex); - switch (lockType) { - - case ReadLock: - // If no body is writting the sprite... - if (!m_write_lock) { - // We can read it - ++m_read_locks; - return true; - } - break; - - case WriteLock: - // If no body is reading and writting... - if (m_read_locks == 0 && !m_write_lock) { - // We can start writting the sprite... - m_write_lock = true; - -#ifdef DEBUG_DOCUMENT_LOCKS - TRACE("DOC: Document::lock: Locked <%d> to write\n", id()); -#endif - return true; - } - break; - - } - } - - if (timeout > 0) { - int delay = MIN(100, timeout); - timeout -= delay; - -#ifdef DEBUG_DOCUMENT_LOCKS - TRACE("DOC: Document::lock: wait 100 msecs for <%d>\n", id()); -#endif - - base::this_thread::sleep_for(double(delay) / 1000.0); - } - else - break; - } - -#ifdef DEBUG_DOCUMENT_LOCKS - TRACE("DOC: Document::lock: Cannot lock <%d> to %s (has %d read locks and %d write locks)\n", - id(), (lockType == ReadLock ? "read": "write"), m_read_locks, m_write_lock); -#endif - - return false; -} - -bool Document::lockToWrite(int timeout) -{ - while (timeout >= 0) { - { - scoped_lock lock(m_mutex); - // this only is possible if there are just one reader - if (m_read_locks == 1) { - ASSERT(!m_write_lock); - m_read_locks = 0; - m_write_lock = true; - -#ifdef DEBUG_DOCUMENT_LOCKS - TRACE("DOC: Document::lockToWrite: Locked <%d> to write\n", id()); -#endif - - return true; - } - } - - if (timeout > 0) { - int delay = MIN(100, timeout); - timeout -= delay; - -#ifdef DEBUG_DOCUMENT_LOCKS - TRACE("DOC: Document::lockToWrite: wait 100 msecs for <%d>\n", id()); -#endif - - base::this_thread::sleep_for(double(delay) / 1000.0); - } - else - break; - } - -#ifdef DEBUG_DOCUMENT_LOCKS - TRACE("DOC: Document::lockToWrite: Cannot lock <%d> to write (has %d read locks and %d write locks)\n", - id(), m_read_locks, m_write_lock); -#endif - - return false; -} - -void Document::unlockToRead() -{ - scoped_lock lock(m_mutex); - - ASSERT(m_read_locks == 0); - ASSERT(m_write_lock); - - m_write_lock = false; - m_read_locks = 1; -} - -void Document::unlock() -{ - scoped_lock lock(m_mutex); - - if (m_write_lock) { - m_write_lock = false; - } - else if (m_read_locks > 0) { - --m_read_locks; - } - else { - ASSERT(false); - } -} - void Document::onContextChanged() { m_undo->setContext(context()); diff --git a/src/app/document.h b/src/app/document.h index 019e27225..5cae82f5a 100644 --- a/src/app/document.h +++ b/src/app/document.h @@ -10,6 +10,7 @@ #include "app/extra_cel.h" #include "app/file/format_options.h" +#include "app/rw_lock.h" #include "app/transformation.h" #include "base/disable_copying.h" #include "base/mutex.h" @@ -50,13 +51,9 @@ namespace app { // An application document. It is the class used to contain one file // opened and being edited by the user (a sprite). - class Document : public doc::Document { + class Document : public doc::Document, + public RWLock { public: - enum LockType { - ReadLock, - WriteLock - }; - Document(Sprite* sprite); ~Document(); @@ -157,23 +154,6 @@ namespace app { void copyLayerContent(const Layer* sourceLayer, Document* destDoc, Layer* destLayer) const; Document* duplicate(DuplicateType type) const; - ////////////////////////////////////////////////////////////////////// - // Multi-threading ("sprite wrappers" use this) - - // Locks the sprite to read or write on it, returning true if the - // sprite can be accessed in the desired mode. - bool lock(LockType lockType, int timeout); - - // If you've locked the sprite to read, using this method you can - // raise your access level to write it. - bool lockToWrite(int timeout); - - // If you've locked the sprite to write, using this method you can - // your access level to only read it. - void unlockToRead(); - - void unlock(); - protected: virtual void onContextChanged() override; @@ -187,15 +167,6 @@ namespace app { // Selected mask region boundaries base::UniquePtr m_maskBoundaries; - // Mutex to modify the 'locked' flag. - base::mutex m_mutex; - - // True if some thread is writing the sprite. - bool m_write_lock; - - // Greater than zero when one or more threads are reading the sprite. - int m_read_locks; - // Data to save the file in the same format that it was loaded base::SharedPtr m_format_options; diff --git a/src/app/document_access.h b/src/app/document_access.h index 6103b28a5..abc041d2b 100644 --- a/src/app/document_access.h +++ b/src/app/document_access.h @@ -1,5 +1,5 @@ // Aseprite -// Copyright (C) 2001-2015 David Capello +// Copyright (C) 2001-2016 David Capello // // This program is distributed under the terms of // the End-User License Agreement for Aseprite. @@ -16,6 +16,7 @@ namespace app { + // TODO remove exceptions and use "DocumentAccess::operator bool()" class LockedDocumentException : public base::Exception { public: LockedDocumentException(const char* msg) throw() @@ -141,7 +142,7 @@ namespace app { , m_from_reader(true) , m_locked(false) { if (m_document) { - if (!m_document->lockToWrite(timeout)) + if (!m_document->upgradeToWrite(timeout)) throw CannotWriteDocumentException(); m_locked = true; @@ -156,7 +157,7 @@ namespace app { void unlock() { if (m_document && m_locked) { if (m_from_reader) - m_document->unlockToRead(); + m_document->downgradeToRead(); else m_document->unlock(); @@ -195,6 +196,42 @@ namespace app { }; + class WeakDocumentReader : public DocumentAccess { + public: + WeakDocumentReader() { + } + + explicit WeakDocumentReader(Document* doc) + : DocumentAccess(doc) + , m_weak_lock(RWLock::WeakUnlocked) { + if (m_document) + m_document->weakLock(&m_weak_lock); + } + + ~WeakDocumentReader() { + weakUnlock(); + } + + bool isLocked() const { + return (m_weak_lock == RWLock::WeakLocked); + } + + protected: + void weakUnlock() { + if (m_document && m_weak_lock != RWLock::WeakUnlocked) { + m_document->weakUnlock(); + m_document = nullptr; + } + } + + private: + // Disable operator= + WeakDocumentReader(const WeakDocumentReader&); + WeakDocumentReader& operator=(const WeakDocumentReader&); + + RWLock::WeakLock m_weak_lock; + }; + } // namespace app #endif diff --git a/src/app/rw_lock.cpp b/src/app/rw_lock.cpp new file mode 100644 index 000000000..667f35606 --- /dev/null +++ b/src/app/rw_lock.cpp @@ -0,0 +1,216 @@ +// Aseprite +// Copyright (C) 2001-2016 David Capello +// +// This program is distributed under the terms of +// the End-User License Agreement for Aseprite. + +#ifdef HAVE_CONFIG_H +#include "config.h" +#endif + +// Uncomment this line in case that you want TRACE() lock/unlock +// operations. +//#define DEBUG_OBJECT_LOCKS + +#include "app/document.h" + +#include "base/scoped_lock.h" +#include "base/thread.h" + +namespace app { + +using namespace base; + +RWLock::RWLock() + : m_write_lock(false) + , m_read_locks(0) + , m_weak_lock(nullptr) +{ +} + +RWLock::~RWLock() +{ + ASSERT(!m_write_lock); + ASSERT(m_read_locks == 0); + ASSERT(m_weak_lock == nullptr); +} + +bool RWLock::lock(LockType lockType, int timeout) +{ + while (timeout >= 0) { + { + scoped_lock lock(m_mutex); + + // Check that there is no weak lock + if (m_weak_lock) { + if (*m_weak_lock == WeakLocked) + *m_weak_lock = WeakUnlocking; + + // Wait some time + if (*m_weak_lock == WeakUnlocking) + goto go_wait; + + ASSERT(*m_weak_lock == WeakUnlocked); + } + + switch (lockType) { + + case ReadLock: + // If no body is writting the object... + if (!m_write_lock) { + // We can read it + ++m_read_locks; + return true; + } + break; + + case WriteLock: + // If no body is reading and writting... + if (m_read_locks == 0 && !m_write_lock) { + // We can start writting the object... + m_write_lock = true; + +#ifdef DEBUG_OBJECT_LOCKS + TRACE("LCK: lock: Locked <%p> to write\n", this); +#endif + return true; + } + break; + + } + + go_wait:; + } + + if (timeout > 0) { + int delay = MIN(100, timeout); + timeout -= delay; + +#ifdef DEBUG_OBJECT_LOCKS + TRACE("LCK: lock: wait 100 msecs for <%p>\n", this); +#endif + + base::this_thread::sleep_for(double(delay) / 1000.0); + } + else + break; + } + +#ifdef DEBUG_OBJECT_LOCKS + TRACE("LCK: lock: Cannot lock <%p> to %s (has %d read locks and %d write locks)\n", + this, (lockType == ReadLock ? "read": "write"), m_read_locks, m_write_lock); +#endif + + return false; +} + +void RWLock::downgradeToRead() +{ + scoped_lock lock(m_mutex); + + ASSERT(m_read_locks == 0); + ASSERT(m_write_lock); + + m_write_lock = false; + m_read_locks = 1; +} + +void RWLock::unlock() +{ + scoped_lock lock(m_mutex); + + if (m_write_lock) { + m_write_lock = false; + } + else if (m_read_locks > 0) { + --m_read_locks; + } + else { + ASSERT(false); + } +} + +bool RWLock::weakLock(WeakLock* weak_lock_flag) +{ + scoped_lock lock(m_mutex); + + if (m_weak_lock || + m_write_lock || + m_read_locks > 0) + return false; + + m_weak_lock = weak_lock_flag; + *m_weak_lock = WeakLocked; + return true; +} + +void RWLock::weakUnlock() +{ + ASSERT(m_weak_lock); + ASSERT(*m_weak_lock != WeakLock::WeakUnlocked); + ASSERT(!m_write_lock); + ASSERT(m_read_locks == 0); + + if (m_weak_lock) { + *m_weak_lock = WeakLock::WeakUnlocked; + m_weak_lock = nullptr; + } +} + +bool RWLock::upgradeToWrite(int timeout) +{ + while (timeout >= 0) { + { + scoped_lock lock(m_mutex); + + // Check that there is no weak lock + if (m_weak_lock) { + if (*m_weak_lock == WeakLocked) + *m_weak_lock = WeakUnlocking; + + // Wait some time + if (*m_weak_lock == WeakUnlocking) + goto go_wait; + + ASSERT(*m_weak_lock == WeakUnlocked); + } + + // this only is possible if there are just one reader + if (m_read_locks == 1) { + ASSERT(!m_write_lock); + m_read_locks = 0; + m_write_lock = true; + +#ifdef DEBUG_OBJECT_LOCKS + TRACE("LCK: upgradeToWrite: Locked <%p> to write\n", this); +#endif + + return true; + } + + go_wait:; + } + + if (timeout > 0) { + int delay = MIN(100, timeout); + timeout -= delay; + +#ifdef DEBUG_OBJECT_LOCKS + TRACE("LCK: upgradeToWrite: wait 100 msecs for <%p>\n", this); +#endif + + base::this_thread::sleep_for(double(delay) / 1000.0); + } + else + break; + } + +#ifdef DEBUG_OBJECT_LOCKS + TRACE("LCK: upgradeToWrite: Cannot lock <%p> to write (has %d read locks and %d write locks)\n", + this, m_read_locks, m_write_lock); +#endif + + return false; +} + +} // namespace app diff --git a/src/app/rw_lock.h b/src/app/rw_lock.h new file mode 100644 index 000000000..bfb5eca9d --- /dev/null +++ b/src/app/rw_lock.h @@ -0,0 +1,79 @@ +// Aseprite +// Copyright (C) 2001-2016 David Capello +// +// This program is distributed under the terms of +// the End-User License Agreement for Aseprite. + +#ifndef APP_RW_LOCK_H_INCLUDED +#define APP_RW_LOCK_H_INCLUDED +#pragma once + +#include "base/disable_copying.h" +#include "base/mutex.h" + +namespace app { + + // A readers-writer lock implementation + class RWLock { + public: + enum LockType { + ReadLock, + WriteLock + }; + + enum WeakLock { + WeakUnlocked, + WeakUnlocking, + WeakLocked, + }; + + RWLock(); + ~RWLock(); + + // Locks the object to read or write on it, returning true if the + // object can be accessed in the desired mode. + bool lock(LockType lockType, int timeout); + + // If you've locked the object to read, using this method you can + // raise your access level to write it. + bool upgradeToWrite(int timeout); + + // If we've locked the object to write, using this method we can + // lower our access to read-only. + void downgradeToRead(); + + // Unlocks a previously successfully lock() operation. + void unlock(); + + // Tries to lock the object for read access in a "weak way" so + // other thread (e.g. UI thread) can lock the object removing this + // weak lock. + // + // The "weak_lock_flag" is used to notify when the "weak lock" is + // lost. + bool weakLock(WeakLock* weak_lock_flag); + void weakUnlock(); + + private: + // Mutex to modify the 'locked' flag. + base::mutex m_mutex; + + // True if some thread is writing the object. + bool m_write_lock; + + // Greater than zero when one or more threads are reading the object. + int m_read_locks; + + // If this isn' nullptr, it means that it points to an unique + // "weak" lock that can be unlocked from other thread. E.g. the + // backup/data recovery thread might weakly lock the object so if + // the user UI thread needs the object again, the backup process + // can stop. + WeakLock* m_weak_lock; + + DISABLE_COPYING(RWLock); + }; + +} // namespace app + +#endif diff --git a/src/app/rw_lock_tests.cpp b/src/app/rw_lock_tests.cpp new file mode 100644 index 000000000..7a593bb71 --- /dev/null +++ b/src/app/rw_lock_tests.cpp @@ -0,0 +1,78 @@ +// Aseprite +// Copyright (C) 2001-2016 David Capello +// +// This program is distributed under the terms of +// the End-User License Agreement for Aseprite. + +#include "tests/test.h" + +#include "app/rw_lock.h" + +using namespace app; + +TEST(RWLock, MultipleReaders) +{ + RWLock a; + EXPECT_TRUE(a.lock(RWLock::ReadLock, 0)); + EXPECT_TRUE(a.lock(RWLock::ReadLock, 0)); + EXPECT_TRUE(a.lock(RWLock::ReadLock, 0)); + EXPECT_TRUE(a.lock(RWLock::ReadLock, 0)); + EXPECT_FALSE(a.lock(RWLock::WriteLock, 0)); + a.unlock(); + a.unlock(); + a.unlock(); + a.unlock(); +} + +TEST(RWLock, OneWriter) +{ + RWLock a; + + EXPECT_TRUE(a.lock(RWLock::WriteLock, 0)); + EXPECT_FALSE(a.lock(RWLock::ReadLock, 0)); + a.unlock(); + + EXPECT_TRUE(a.lock(RWLock::ReadLock, 0)); + EXPECT_FALSE(a.lock(RWLock::WriteLock, 0)); + a.unlock(); + + EXPECT_TRUE(a.lock(RWLock::ReadLock, 0)); + EXPECT_TRUE(a.lock(RWLock::ReadLock, 0)); + EXPECT_FALSE(a.lock(RWLock::WriteLock, 0)); + a.unlock(); + EXPECT_FALSE(a.lock(RWLock::WriteLock, 0)); + a.unlock(); + EXPECT_TRUE(a.lock(RWLock::WriteLock, 0)); + EXPECT_FALSE(a.lock(RWLock::WriteLock, 0)); + a.unlock(); +} + +TEST(RWLock, UpgradeToWrite) +{ + RWLock a; + + EXPECT_TRUE(a.lock(RWLock::ReadLock, 0)); + EXPECT_FALSE(a.lock(RWLock::WriteLock, 0)); + EXPECT_TRUE(a.upgradeToWrite(0)); + EXPECT_FALSE(a.lock(RWLock::ReadLock, 0)); + EXPECT_FALSE(a.lock(RWLock::WriteLock, 0)); + a.downgradeToRead(); + a.unlock(); +} + +TEST(RWLock, WeakLock) +{ + RWLock a; + RWLock::WeakLock flag = RWLock::WeakUnlocked; + + EXPECT_TRUE(a.weakLock(&flag)); + EXPECT_EQ(RWLock::WeakLocked, flag); + EXPECT_FALSE(a.lock(RWLock::ReadLock, 0)); + EXPECT_EQ(RWLock::WeakUnlocking, flag); + a.weakUnlock(); + EXPECT_EQ(RWLock::WeakUnlocked, flag); + + EXPECT_TRUE(a.lock(RWLock::ReadLock, 0)); + EXPECT_FALSE(a.weakLock(&flag)); + a.unlock(); +} diff --git a/src/app/ui/editor/editor.cpp b/src/app/ui/editor/editor.cpp index a26b7ecf8..3cb888b72 100644 --- a/src/app/ui/editor/editor.cpp +++ b/src/app/ui/editor/editor.cpp @@ -1464,8 +1464,9 @@ void Editor::onPaint(ui::PaintEvent& ev) // Editor with sprite else { try { - // Lock the sprite to read/render it. - DocumentReader documentReader(m_document, 0); + // Lock the sprite to read/render it. We wait 1/4 secs in case + // the background thread is making a backup. + DocumentReader documentReader(m_document, 250); // Draw the sprite in the editor renderChrono.reset(); diff --git a/src/app/ui/timeline.cpp b/src/app/ui/timeline.cpp index 9b1ef78c2..ca9216c7b 100644 --- a/src/app/ui/timeline.cpp +++ b/src/app/ui/timeline.cpp @@ -1140,8 +1140,9 @@ void Timeline::onPaint(ui::PaintEvent& ev) goto paintNoDoc; try { - // Lock the sprite to read/render it. - const DocumentReader documentReader(m_document, 0); + // Lock the sprite to read/render it. We wait 1/4 secs in case + // the background thread is making a backup. + const DocumentReader documentReader(m_document, 250); layer_t layer, firstLayer, lastLayer; frame_t frame, firstFrame, lastFrame; diff --git a/src/doc/cancel_io.h b/src/doc/cancel_io.h new file mode 100644 index 000000000..f26277295 --- /dev/null +++ b/src/doc/cancel_io.h @@ -0,0 +1,21 @@ +// Aseprite Document Library +// Copyright (c) 2001-2016 David Capello +// +// This file is released under the terms of the MIT license. +// Read LICENSE.txt for more information. + +#ifndef DOC_CANCEL_IO_H_INCLUDED +#define DOC_CANCEL_IO_H_INCLUDED +#pragma once + +namespace doc { + + class CancelIO { + public: + virtual ~CancelIO() { } + virtual bool isCanceled() = 0; + }; + +} // namespace doc + +#endif diff --git a/src/doc/image_io.cpp b/src/doc/image_io.cpp index 264e17023..32fafe342 100644 --- a/src/doc/image_io.cpp +++ b/src/doc/image_io.cpp @@ -14,6 +14,7 @@ #include "base/exception.h" #include "base/serialization.h" #include "base/unique_ptr.h" +#include "doc/cancel_io.h" #include "doc/image.h" #include "zlib.h" @@ -26,7 +27,7 @@ using namespace base::serialization::little_endian; // TODO Create a zlib wrapper for iostreams -void write_image(std::ostream& os, const Image* image) +bool write_image(std::ostream& os, const Image* image, CancelIO* cancel) { write32(os, image->id()); write8(os, image->pixelFormat()); // Pixel format @@ -57,6 +58,11 @@ void write_image(std::ostream& os, const Image* image) int total_output_bytes = 0; for (int y=0; yheight(); y++) { + if (cancel && cancel->isCanceled()) { + deflateEnd(&zstream); + return false; + } + zstream.next_in = (Bytef*)image->getPixelAddress(0, y); zstream.avail_in = rowSize; int flush = (y == image->height()-1 ? Z_FINISH: Z_NO_FLUSH); @@ -90,6 +96,7 @@ void write_image(std::ostream& os, const Image* image) os.seekp(bak); } #endif + return true; } Image* read_image(std::istream& is, bool setId) diff --git a/src/doc/image_io.h b/src/doc/image_io.h index a1abef434..8145bdf0b 100644 --- a/src/doc/image_io.h +++ b/src/doc/image_io.h @@ -1,5 +1,5 @@ // Aseprite Document Library -// Copyright (c) 2001-2015 David Capello +// Copyright (c) 2001-2016 David Capello // // This file is released under the terms of the MIT license. // Read LICENSE.txt for more information. @@ -12,9 +12,10 @@ namespace doc { + class CancelIO; class Image; - void write_image(std::ostream& os, const Image* image); + bool write_image(std::ostream& os, const Image* image, CancelIO* cancel = nullptr); Image* read_image(std::istream& is, bool setId = true); } // namespace doc