diff --git a/src/app/CMakeLists.txt b/src/app/CMakeLists.txt index 3d129d41d..3525784d7 100644 --- a/src/app/CMakeLists.txt +++ b/src/app/CMakeLists.txt @@ -324,6 +324,7 @@ add_library(app-lib res/palettes_loader_delegate.cpp res/resources_loader.cpp resource_finder.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 fd4f52724..74b7ef82c 100644 --- a/src/app/context_flags.cpp +++ b/src/app/context_flags.cpp @@ -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. @@ -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/document.cpp b/src/app/document.cpp index 6d2f23da2..1402e65bb 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 @@ -439,130 +430,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..1e4f68cc6 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(); diff --git a/src/app/rw_lock.cpp b/src/app/rw_lock.cpp new file mode 100644 index 000000000..c7864db71 --- /dev/null +++ b/src/app/rw_lock.cpp @@ -0,0 +1,157 @@ +// 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) +{ +} + +RWLock::~RWLock() +{ + ASSERT(!m_write_lock); + ASSERT(m_read_locks == 0); +} + +bool RWLock::lock(LockType lockType, int timeout) +{ + while (timeout >= 0) { + { + scoped_lock lock(m_mutex); + 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; + + } + } + + 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; +} + +bool RWLock::upgradeToWrite(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_OBJECT_LOCKS + TRACE("LCK: upgradeToWrite: Locked <%p> to write\n", this); +#endif + + return true; + } + } + + 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; +} + +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); + } +} + +} // namespace app diff --git a/src/app/rw_lock.h b/src/app/rw_lock.h new file mode 100644 index 000000000..7497bd8e7 --- /dev/null +++ b/src/app/rw_lock.h @@ -0,0 +1,57 @@ +// 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 + }; + + 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(); + + 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; + + 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..3e21f8363 --- /dev/null +++ b/src/app/rw_lock_tests.cpp @@ -0,0 +1,61 @@ +// 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(); +}