Improve write/read stability of backup data

Now we backup 3 versions for each object. And when an object is saved
correctly on disk, a "magic number" is saved at the beginning of the file.
In this way we know that the file really represents the full object (and
a crash was not occurred in the middle of the process).

src/app/crash/internals.h

@@ -0,0 +1,63 @@
+// Aseprite
+// Copyright (C) 2001-2015  David Capello
+//
+// This program is free software; you can redistribute it and/or modify
+// it under the terms of the GNU General Public License version 2 as
+// published by the Free Software Foundation.
+
+#ifndef APP_CRASH_INTERNALS_H_INCLUDED
+#define APP_CRASH_INTERNALS_H_INCLUDED
+#pragma once
+
+#include "doc/object.h"
+
+#include <algorithm>
+#include <functional>
+#include <map>
+
+namespace app {
+namespace crash {
+
+  const uint32_t MAGIC_NUMBER = 0x454E4946; // 'FINE' in ASCII
+
+  class ObjVersions {
+  public:
+    ObjVersions() {
+      m_vers[0] = 0;
+      m_vers[1] = 0;
+      m_vers[2] = 0;
+    }
+
+    size_t size() const { return 3; }
+    doc::ObjectVersion operator[](int i) const { return m_vers[i]; }
+
+    doc::ObjectVersion newer() { return m_vers[0]; }
+    doc::ObjectVersion older() { return m_vers[2]; }
+
+    // Adds a newer version
+    void rotateRevisions(doc::ObjectVersion newer) {
+      // Rotate versions
+      m_vers[2] = m_vers[1];
+      m_vers[1] = m_vers[0];
+      m_vers[0] = newer;
+    }
+
+    // Adds a version (we don't know if the version if the latest one)
+    void add(doc::ObjectVersion ver) {
+      auto minver = std::min_element(m_vers, m_vers+2);
+      if (*minver < ver) {
+        *minver = ver;
+        std::sort(m_vers, m_vers+2, std::greater<doc::ObjectVersion>());
+      }
+    }
+
+  private:
+    doc::ObjectVersion m_vers[3];
+  };
+
+  typedef std::map<doc::ObjectId, ObjVersions> ObjVersionsMap;
+
+} // namespace crash
+} // namespace app
+
+#endif

src/app/crash/read_document.cpp

@@ -12,6 +12,7 @@
 #include "app/crash/read_document.h"
 
 #include "app/console.h"
+#include "app/crash/internals.h"
 #include "app/document.h"
 #include "base/convert_to.h"
 #include "base/exception.h"
@@ -59,7 +60,61 @@ class Reader : public SubObjectsIO {
 public:
   Reader(const std::string& dir)
     : m_sprite(nullptr)
-    , m_dir(dir) {
+    , m_dir(dir)
+    , m_docId(0)
+    , m_docVersions(nullptr)
+    , m_loadInfo(nullptr) {
+    for (const auto& fn : base::list_files(dir)) {
+      auto i = fn.find('-');
+      if (i == std::string::npos)
+        continue;               // Has no ID
+
+      auto j = fn.find('.', ++i);
+      if (j == std::string::npos)
+        continue;               // Has no version
+
+      ObjectId id = base::convert_to<int>(fn.substr(i, j - i));
+      ObjectVersion ver = base::convert_to<int>(fn.substr(j+1));
+      if (!id || !ver)
+        continue;               // Error converting strings to ID/ver
+
+      ObjVersions& versions = m_objVersions[id];
+      versions.add(ver);
+
+      if (fn.compare(0, 3, "doc") == 0) {
+        if (!m_docId)
+          m_docId = id;
+        else {
+          ASSERT(m_docId == id);
+        }
+
+        m_docVersions = &versions;
+      }
+    }
+  }
+
+  app::Document* loadDocument() {
+    app::Document* doc = loadObject<app::Document*>("doc", m_docId, &Reader::readDocument);
+    if (!doc)
+      Console().printf("Error recovering the document\n");
+    return doc;
+  }
+
+  bool loadDocumentInfo(DocumentInfo& info) {
+    m_loadInfo = &info;
+    return
+      loadObject<app::Document*>("doc", m_docId, &Reader::readDocument)
+      == (app::Document*)1;
+  }
+
+private:
+
+  const ObjectVersion docId() const {
+    return m_docId;
+  }
+
+  const ObjVersions* docVersions() const {
+    return m_docVersions;
   }
 
   Sprite* loadSprite(ObjectId sprId) {
@@ -82,24 +137,61 @@ public:
     return m_celdatas[celdataId] = celData;
   }
 
-private:
   template<typename T>
   T loadObject(const char* prefix, ObjectId id, T (Reader::*readMember)(std::ifstream&)) {
-    TRACE(" - Restoring %s%d\n", prefix, id);
+    const ObjVersions& versions = m_objVersions[id];
 
-    IFSTREAM(m_dir, s, prefix + base::convert_to<std::string>(id));
+    for (size_t i=0; i<versions.size(); ++i) {
+      ObjectVersion ver = versions[i];
+      if (!ver)
+        continue;
 
-    T obj = nullptr;
+      TRACE(" - Restoring %s #%d v%d\n", prefix, id, ver);
-    if (s.good())
-      obj = (this->*readMember)(s);
 
-    if (obj) {
+      std::string fn = prefix;
-      TRACE(" - %s%d restored successfully\n", prefix, id);
+      fn.push_back('-');
-      return obj;
+      fn += base::convert_to<std::string>(id);
+      fn.push_back('.');
+      fn += base::convert_to<std::string>(ver);
+
+      IFSTREAM(m_dir, s, fn);
+      T obj = nullptr;
+      if (read32(s) == MAGIC_NUMBER)
+        obj = (this->*readMember)(s);
+
+      if (obj) {
+        TRACE(" - %s #%d v%d restored successfully\n", prefix, id, ver);
+        return obj;
+      }
+      else {
+        TRACE(" - %s #%d v%d was not restored\n", prefix, id, ver);
+        if (!m_loadInfo)
+          Console().printf("Error loading object %s #%d v%d\n", prefix, id, ver);
+      }
+    }
+
+    return nullptr;
+  }
+
+  app::Document* readDocument(std::ifstream& s) {
+    ObjectId sprId = read32(s);
+    std::string filename = read_string(s);
+
+    // Load DocumentInfo only
+    if (m_loadInfo) {
+      m_loadInfo->filename = filename;
+      return (app::Document*)loadSprite(sprId);
+    }
+
+    Sprite* spr = loadSprite(sprId);
+    if (spr) {
+      app::Document* doc = new app::Document(spr);
+      doc->setFilename(filename);
+      doc->impossibleToBackToSavedState();
+      return doc;
     }
     else {
-      TRACE(" - %s%d was not restored\n", prefix, id);
+      Console().printf("Unable to load sprite #%d\n", sprId);
-      Console().printf("Error loading object %s%d\n", prefix, id);
       return nullptr;
     }
   }
@@ -109,25 +201,34 @@ private:
     int w = read16(s);
     int h = read16(s);
     color_t transparentColor = read32(s);
+    frame_t nframes = read32(s);
 
     if (format != IMAGE_RGB &&
         format != IMAGE_INDEXED &&
         format != IMAGE_GRAYSCALE) {
-      Console().printf("Invalid sprite format #%d\n", (int)format);
+      if (!m_loadInfo)
+        Console().printf("Invalid sprite format #%d\n", (int)format);
       return nullptr;
     }
 
     if (w < 1 || h < 1 || w > 0xfffff || h > 0xfffff) {
-      Console().printf("Invalid sprite dimension %dx%d\n", w, h);
+      if (!m_loadInfo)
+        Console().printf("Invalid sprite dimension %dx%d\n", w, h);
       return nullptr;
     }
 
+    if (m_loadInfo) {
+      m_loadInfo->format = format;
+      m_loadInfo->width = w;
+      m_loadInfo->height = w;
+      m_loadInfo->frames = nframes;
+      return (Sprite*)1;
+    }
+
     base::UniquePtr<Sprite> spr(new Sprite(format, w, h, 256));
     m_sprite = spr.get();
     spr->setTransparentColor(transparentColor);
 
-    // Frame durations
-    frame_t nframes = read32(s);
     if (nframes >= 1) {
       spr->setTotalFrames(nframes);
       for (frame_t fr=0; fr<nframes; ++fr) {
@@ -214,6 +315,10 @@ private:
 
   Sprite* m_sprite;    // Used to pass the sprite in LayerImage() ctor
   std::string m_dir;
+  ObjectVersion m_docId;
+  ObjVersionsMap m_objVersions;
+  ObjVersions* m_docVersions;
+  DocumentInfo* m_loadInfo;
   std::map<ObjectId, ImageRef> m_images;
   std::map<ObjectId, CelDataRef> m_celdatas;
 };
@@ -225,46 +330,12 @@ private:
 
 bool read_document_info(const std::string& dir, DocumentInfo& info)
 {
-  ObjectId sprId;
+  return Reader(dir).loadDocumentInfo(info);
-
-  if (base::is_file(base::join_path(dir, "doc"))) {
-    IFSTREAM(dir, s, "doc");
-    sprId = read32(s);
-    info.filename = read_string(s);
-  }
-  else
-    return false;
-
-  IFSTREAM(dir, s, "spr" + base::convert_to<std::string>(sprId));
-  info.format = (PixelFormat)read8(s);
-  info.width = read16(s);
-  info.height = read16(s);
-  read32(s);                    // Ignore transparent color
-  info.frames = read32(s);
-  return true;
 }
 
 app::Document* read_document(const std::string& dir)
 {
-  if (!base::is_file(base::join_path(dir, "doc")))
+  return Reader(dir).loadDocument();
-    return nullptr;
-
-  IFSTREAM(dir, s, "doc");
-  ObjectId sprId = read32(s);
-  std::string filename = read_string(s);
-
-  Reader reader(dir);
-  Sprite* spr = reader.loadSprite(sprId);
-  if (spr) {
-    app::Document* doc = new app::Document(spr);
-    doc->setFilename(filename);
-    doc->impossibleToBackToSavedState();
-    return doc;
-  }
-  else {
-    Console().printf("Unable to load sprite #%d\n", sprId);
-    return nullptr;
-  }
 }
 
 } // namespace crash

src/app/crash/read_document.h

@@ -24,6 +24,13 @@ namespace crash {
     int height;
     doc::frame_t frames;
     std::string filename;
+
+    DocumentInfo() :
+      format(doc::IMAGE_RGB),
+      width(0),
+      height(0),
+      frames(0) {
+    }
   };
 
   bool read_document_info(const std::string& dir, DocumentInfo& info);

src/app/crash/write_document.cpp

@@ -11,6 +11,7 @@
 
 #include "app/crash/write_document.h"
 
+#include "app/crash/internals.h"
 #include "app/document.h"
 #include "base/convert_to.h"
 #include "base/fs.h"
@@ -43,34 +44,30 @@ using namespace base::serialization::little_endian;
 using namespace doc;
 
 #ifdef _WIN32
-  #define OFSTREAM(dir, name, fn) \
+  #define OFSTREAM(name, fullfn) \
-    std::ofstream name(base::from_utf8(base::join_path(dir, fn)), std::ofstream::binary);
+    std::ofstream name(base::from_utf8(fullfn), std::ofstream::binary);
 #else
-  #define OFSTREAM(dir, name, fn) \
+  #define OFSTREAM(name, fullfn) \
-    std::ofstream name(base::join_path(dir, fn).c_str(), std::ofstream::binary);
+    std::ofstream name(fullfn.c_str(), std::ofstream::binary);
 #endif
 
 namespace {
 
-typedef std::map<ObjectId, ObjectVersion> Versions;
+static std::map<ObjectId, ObjVersionsMap> g_docVersions;
-static std::map<ObjectId, Versions> g_documentObjects;
 
 class Writer {
 public:
   Writer(const std::string& dir, app::Document* doc)
     : m_dir(dir)
     , m_doc(doc)
-    , m_versions(g_documentObjects[doc->id()]) {
+    , m_objVersions(g_docVersions[doc->id()]) {
   }
 
   void saveDocument() {
     Sprite* spr = m_doc->sprite();
 
-    saveObject(nullptr, m_doc, &Writer::writeDocumentFile);
+    // Save from objects without children (e.g. images), to aggregated
-    saveObject("spr", spr, &Writer::writeSprite);
+    // objects (e.g. cels, layers, etc.)
-
-    //////////////////////////////////////////////////////////////////////
-    // Create one stream for each object
 
     for (Palette* pal : spr->getPalettes())
       saveObject("pal", pal, &Writer::writePalette);
@@ -78,19 +75,21 @@ public:
     for (FrameTag* frtag : spr->frameTags())
       saveObject("frtag", frtag, &Writer::writeFrameTag);
 
+    for (Cel* cel : spr->uniqueCels()) {
+      saveObject("img", cel->image(), &Writer::writeImage);
+      saveObject("celdata", cel->data(), &Writer::writeCelData);
+    }
+
+    for (Cel* cel : spr->cels())
+      saveObject("cel", cel, &Writer::writeCel);
+
     std::vector<Layer*> layers;
     spr->getLayersList(layers);
     for (Layer* lay : layers)
       saveObject("lay", lay, &Writer::writeLayerStructure);
 
-    for (Cel* cel : spr->cels())
+    saveObject("spr", spr, &Writer::writeSprite);
-      saveObject("cel", cel, &Writer::writeCel);
+    saveObject("doc", m_doc, &Writer::writeDocumentFile);
-
-    // Images (CelData)
-    for (Cel* cel : spr->uniqueCels()) {
-      saveObject("celdata", cel->data(), &Writer::writeCelData);
-      saveObject("img", cel->image(), &Writer::writeImage);
-    }
   }
 
 private:
@@ -167,25 +166,44 @@ private:
     if (!obj->version())
       obj->incrementVersion();
 
-    if (m_versions[obj->id()] != obj->version()) {
+    ObjVersions& versions = m_objVersions[obj->id()];
-      std::string fn = (prefix ? prefix + base::convert_to<std::string>(obj->id()): "doc");
+    if (versions.newer() == obj->version())
+      return;
 
-      OFSTREAM(m_dir, s, fn);
+    std::string fn = prefix;
-      (this->*writeMember)(s, obj);
+    fn.push_back('-');
-      m_versions[obj->id()] = obj->version();
+    fn += base::convert_to<std::string>(obj->id());
 
-      TRACE(" - %s %d saved with version %d\n",
+    std::string fullfn = base::join_path(m_dir, fn);
-            prefix, obj->id(), obj->version());
+    std::string oldfn = fullfn + "." + base::convert_to<std::string>(versions.older());
+    fullfn += "." + base::convert_to<std::string>(obj->version());
+
+    OFSTREAM(s, fullfn);
+    write32(s, 0);                // Leave a room for the magic number
+    (this->*writeMember)(s, obj); // Write the object
+
+    // Write the magic number
+    s.seekp(0);
+    write32(s, MAGIC_NUMBER);
+
+    // Remove the older version
+    try {
+      if (versions.older() && base::is_file(oldfn))
+        base::delete_file(oldfn);
     }
-    else {
+    catch (const std::exception&) {
-      TRACE(" - Ignoring %s %d (version %d already saved)\n",
+      TRACE(" - Cannot delete %s #%d v%d\n", prefix, obj->id(), versions.older());
-            prefix, obj->id(), obj->version());
     }
+
+    // Rotate versions and add the latest one
+    versions.rotateRevisions(obj->version());
+
+    TRACE(" - Saved %s #%d v%d\n", prefix, obj->id(), obj->version());
   }
 
   std::string m_dir;
   app::Document* m_doc;
-  Versions& m_versions;
+  ObjVersionsMap& m_objVersions;
 };
 
 } // anonymous namespace
@@ -202,12 +220,12 @@ void write_document(const std::string& dir, app::Document* doc)
 void delete_document_internals(app::Document* doc)
 {
   ASSERT(doc);
-  auto it = g_documentObjects.find(doc->id());
+  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_documentObjects.end())
+  if (it != g_docVersions.end())
-    g_documentObjects.erase(it);
+    g_docVersions.erase(it);
 }
 
 } // namespace crash