Add timeout parameter to Context/Document lock/unlock operations

src/app/commands/cmd_import_sprite_sheet.cpp

@@ -169,7 +169,7 @@ private:
       releaseEditor();
 
       if (m_fileOpened) {
-        DocumentDestroyer destroyer(m_context, oldDocument);
+        DocumentDestroyer destroyer(m_context, oldDocument, 100);
         destroyer.destroyDocument();
       }
     }

src/app/context_access.h

@@ -45,17 +45,17 @@ namespace app {
     }
 
   protected:
-    ContextAccess(const Context* context)
+    ContextAccess(const Context* context, int timeout)
       : m_context(context)
-      , m_document(context->activeDocument())
+      , m_document(context->activeDocument(), timeout)
       , m_location(context->activeLocation())
     {
     }
 
     template<typename DocumentReaderT>
-    ContextAccess(const Context* context, const DocumentReaderT& documentReader)
+    ContextAccess(const Context* context, const DocumentReaderT& documentReader, int timeout)
       : m_context(context)
-      , m_document(documentReader)
+      , m_document(documentReader, timeout)
       , m_location(context->activeLocation())
     {
     }
@@ -70,8 +70,8 @@ namespace app {
   // active document.
   class ContextReader : public ContextAccess<DocumentReader> {
   public:
-    ContextReader(const Context* context)
+    ContextReader(const Context* context, int timeout = 0)
-      : ContextAccess<DocumentReader>(context) {
+      : ContextAccess<DocumentReader>(context, timeout) {
     }
   };
 
@@ -79,12 +79,12 @@ namespace app {
   // active document.
   class ContextWriter : public ContextAccess<DocumentWriter> {
   public:
-    ContextWriter(const Context* context)
+    ContextWriter(const Context* context, int timeout = 0)
-      : ContextAccess<DocumentWriter>(context) {
+      : ContextAccess<DocumentWriter>(context, timeout) {
     }
 
-    ContextWriter(const ContextReader& reader)
+    ContextWriter(const ContextReader& reader, int timeout = 0)
-      : ContextAccess<DocumentWriter>(reader.context(), reader.document()) {
+      : ContextAccess<DocumentWriter>(reader.context(), reader.document(), timeout) {
     }
   };
 

src/app/context_flags.cpp

@@ -35,7 +35,7 @@ void ContextFlags::update(Context* context)
   if (document) {
     m_flags |= HasActiveDocument;
 
-    if (document->lock(Document::ReadLock)) {
+    if (document->lock(Document::ReadLock, 0)) {
       m_flags |= ActiveDocumentIsReadable;
 
       if (document->isMaskVisible())
@@ -75,7 +75,7 @@ void ContextFlags::update(Context* context)
         }
       }
 
-      if (document->lockToWrite())
+      if (document->lockToWrite(0))
         m_flags |= ActiveDocumentIsWritable;
 
       document->unlock();

src/app/crash/session.cpp

@@ -134,8 +134,8 @@ void Session::removeFromDisk()
 
 void Session::saveDocumentChanges(app::Document* doc)
 {
-  DocumentReader reader(doc);
+  DocumentReader reader(doc, 250);
-  DocumentWriter writer(reader);
+  DocumentWriter writer(reader, 250);
   app::Context ctx;
   std::string dir = base::join_path(m_path,
     base::convert_to<std::string>(doc->id()));

src/app/document.cpp

@@ -23,6 +23,7 @@
 #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"
@@ -510,31 +511,42 @@ Document* Document::duplicate(DuplicateType type) const
 //////////////////////////////////////////////////////////////////////
 // Multi-threading ("sprite wrappers" use this)
 
-bool Document::lock(LockType lockType)
+bool Document::lock(LockType lockType, int timeout)
 {
-  scoped_lock lock(*m_mutex);
+  while (timeout >= 0) {
+    {
+      scoped_lock lock(*m_mutex);
+      switch (lockType) {
 
-  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;
+            TRACE("Document::lock: Locked <%d> to write\n", id());
+            return true;
+          }
+          break;
 
-    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 (timeout > 0) {
-      // If no body is reading and writting...
+      int delay = MIN(100, timeout);
-      if (m_read_locks == 0 && !m_write_lock) {
+      timeout -= delay;
-        // We can start writting the sprite...
+      base::this_thread::sleep_for(double(delay) / 1000.0);
-        m_write_lock = true;
+    }
-        TRACE("Document::lock: Locked <%d> to write\n", id());
+    else
-        return true;
-      }
       break;
-
   }
 
   TRACE("Document::lock: Cannot lock <%d> to %s (has %d read locks and %d write locks)\n",
@@ -542,23 +554,27 @@ bool Document::lock(LockType lockType)
   return false;
 }
 
-bool Document::lockToWrite()
+bool Document::lockToWrite(int timeout)
 {
-  scoped_lock lock(*m_mutex);
+  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;
+        TRACE("Document::lockToWrite: Locked <%d> to write\n", id());
+        return true;
+      }
+    }
+    timeout -= 100;
+    base::this_thread::sleep_for(0.100);
+  }
 
-  // this only is possible if there are just one reader
+  TRACE("Document::lockToWrite: Cannot lock <%d> to write (has %d read locks and %d write locks)\n",
-  if (m_read_locks == 1) {
+    id(), m_read_locks, m_write_lock);
-    ASSERT(!m_write_lock);
+  return false;
-    m_read_locks = 0;
-    m_write_lock = true;
-    TRACE("Document::lockToWrite: Locked <%d> to write\n", id());
-    return true;
-  }
-  else {
-    TRACE("Document::lockToWrite: Cannot lock <%d> to write (has %d read locks and %d write locks)\n",
-      id(), m_read_locks, m_write_lock);
-    return false;
-  }
 }
 
 void Document::unlockToRead()

src/app/document.h

@@ -170,11 +170,11 @@ namespace app {
 
     // 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);
+    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();
+    bool lockToWrite(int timeout);
 
     // If you've locked the sprite to write, using this method you can
     // your access level to only read it.

src/app/document_access.h

@@ -69,35 +69,20 @@ namespace app {
     {
     }
 
-    explicit DocumentReader(Document* document)
+    explicit DocumentReader(Document* document, int timeout)
       : DocumentAccess(document)
     {
-      if (m_document && !m_document->lock(Document::ReadLock))
+      if (m_document && !m_document->lock(Document::ReadLock, timeout))
         throw LockedDocumentException();
     }
 
-    explicit DocumentReader(const DocumentReader& copy)
+    explicit DocumentReader(const DocumentReader& copy, int timeout)
       : DocumentAccess(copy)
     {
-      if (m_document && !m_document->lock(Document::ReadLock))
+      if (m_document && !m_document->lock(Document::ReadLock, timeout))
         throw LockedDocumentException();
     }
 
-    DocumentReader& operator=(const DocumentReader& copy)
-    {
-      // unlock old document
-      if (m_document)
-        m_document->unlock();
-
-      DocumentAccess::operator=(copy);
-
-      // relock the document
-      if (m_document && !m_document->lock(Document::ReadLock))
-        throw LockedDocumentException();
-
-      return *this;
-    }
-
     ~DocumentReader()
     {
       // unlock the document
@@ -105,6 +90,9 @@ namespace app {
         m_document->unlock();
     }
 
+  private:
+    // Disable operator=
+    DocumentReader& operator=(const DocumentReader&);
   };
 
   // Class to modify the document's state. Its constructor request a
@@ -120,13 +108,13 @@ namespace app {
     {
     }
 
-    explicit DocumentWriter(Document* document)
+    explicit DocumentWriter(Document* document, int timeout)
       : DocumentAccess(document)
       , m_from_reader(false)
       , m_locked(false)
     {
       if (m_document) {
-        if (!m_document->lock(Document::WriteLock))
+        if (!m_document->lock(Document::WriteLock, timeout))
           throw LockedDocumentException();
 
         m_locked = true;
@@ -135,13 +123,13 @@ namespace app {
 
     // Constructor that can be used to elevate the given reader-lock to
     // writer permission.
-    explicit DocumentWriter(const DocumentReader& document)
+    explicit DocumentWriter(const DocumentReader& document, int timeout)
       : DocumentAccess(document)
       , m_from_reader(true)
       , m_locked(false)
     {
       if (m_document) {
-        if (!m_document->lockToWrite())
+        if (!m_document->lockToWrite(timeout))
           throw LockedDocumentException();
 
         m_locked = true;
@@ -153,24 +141,6 @@ namespace app {
       unlockWriter();
     }
 
-    DocumentWriter& operator=(const DocumentReader& copy)
-    {
-      unlockWriter();
-
-      DocumentAccess::operator=(copy);
-
-      if (m_document) {
-        m_from_reader = true;
-
-        if (!m_document->lockToWrite())
-          throw LockedDocumentException();
-
-        m_locked = true;
-      }
-
-      return *this;
-    }
-
   protected:
 
     void unlockWriter()
@@ -191,13 +161,14 @@ namespace app {
     // Non-copyable
     DocumentWriter(const DocumentWriter&);
     DocumentWriter& operator=(const DocumentWriter&);
+    DocumentWriter& operator=(const DocumentReader&);
   };
 
   // Used to destroy the active document in the context.
   class DocumentDestroyer : public DocumentWriter {
   public:
-    explicit DocumentDestroyer(Context* context, Document* document)
+    explicit DocumentDestroyer(Context* context, Document* document, int timeout)
-      : DocumentWriter(document)
+      : DocumentWriter(document, timeout)
     {
     }
 

src/app/ui/document_view.cpp

@@ -290,7 +290,7 @@ bool DocumentView::onCloseView(Workspace* workspace)
 
   // Destroy the sprite (locking it as writer)
   DocumentDestroyer destroyer(
-    static_cast<app::Context*>(m_document->context()), m_document);
+    static_cast<app::Context*>(m_document->context()), m_document, 250);
 
   StatusBar::instance()
     ->setStatusText(0, "Sprite '%s' closed.",

src/app/ui/editor/editor.cpp

@@ -1318,7 +1318,7 @@ void Editor::onPaint(ui::PaintEvent& ev)
   else {
     try {
       // Lock the sprite to read/render it.
-      DocumentReader documentReader(m_document);
+      DocumentReader documentReader(m_document, 250);
 
       // Draw the sprite in the editor
       drawSpriteUnclippedRect(g, gfx::Rect(0, 0, m_sprite->width(), m_sprite->height()));

src/app/ui/editor/tool_loop_impl.cpp

@@ -191,20 +191,13 @@ public:
     if (!m_canceled) {
       // Paint ink
       if (getInk()->isPaint()) {
-        for (int i=0; ; ++i) {
+        try {
-          // TODO add a "wait_n_seconds" parameter to ContextReader/Writer
+          ContextReader reader(m_context, 500);
-          try {
+          ContextWriter writer(reader, 500);
-            ContextReader reader(m_context);
+          m_expandCelCanvas.commit();
-            ContextWriter writer(reader);
+        }
-            m_expandCelCanvas.commit();
+        catch (const LockedDocumentException& ex) {
-            break;
+          Console::showException(ex);
-          }
-          catch (const LockedDocumentException& ex) {
-            if (i == 10)
-              Console::showException(ex);
-            else
-              base::this_thread::sleep_for(0.10);
-          }
         }
       }
       // Selection ink
@@ -220,19 +213,13 @@ public:
 
     // If the trace was canceled or it is not a 'paint' ink...
     if (m_canceled || !getInk()->isPaint()) {
-      for (int i=0; ; ++i) {
+      try {
-        try {
+        ContextReader reader(m_context, 500);
-          ContextReader reader(m_context);
+        ContextWriter writer(reader, 500);
-          ContextWriter writer(reader);
+        m_expandCelCanvas.rollback();
-          m_expandCelCanvas.rollback();
+      }
-          break;
+      catch (const LockedDocumentException& ex) {
-        }
+        Console::showException(ex);
-        catch (const LockedDocumentException& ex) {
-          if (i == 10)
-            Console::showException(ex);
-          else
-            base::this_thread::sleep_for(0.10);
-        }
       }
     }
 

src/app/ui/status_bar.cpp

@@ -450,7 +450,7 @@ void StatusBar::updateFromDocument(Editor* editor)
 {
   try {
     if (editor && editor->document()) {
-      const DocumentReader reader(editor->document());
+      const DocumentReader reader(editor->document(), 100);
       m_commandsBox->setVisible(true);
 
       // Cel opacity

src/app/ui/timeline.cpp

@@ -809,7 +809,7 @@ void Timeline::onPaint(ui::PaintEvent& ev)
 
   try {
     // Lock the sprite to read/render it.
-    const DocumentReader documentReader(m_document);
+    const DocumentReader documentReader(m_document, 250);
 
     LayerIndex layer, first_layer, last_layer;
     frame_t frame, first_frame, last_frame;