From 9ed8d704ddb98775ff2651f5a02b64a67e129a17 Mon Sep 17 00:00:00 2001
From: Jakob Unterwurzacher <jakobunt@gmail.com>
Date: Mon, 24 Jul 2017 22:34:01 +0200
Subject: [PATCH] eraseNode: erase the right FileNode

When a file is opened twice concurrently, we can end up with
two separate FileNodes for a single path.

This seems to be the root cause for the crashes reported under
https://github.com/vgough/encfs/issues/214 , as commit

  af64702dd0c5be3a2434d670f9e3dbb803f3e388
  "Simplify FileNode Cache. Remove the need for PlaceHolder."

removed the awareness for different FileNodes.

The crashes have been fixed by

  e2f0f8e3c68604a46d8665c3b0125bb35a0d9181
  "fuseFhMap: translate FUSE file handles to FileNode pointers"

at the cost of introducing a memory leak. One of the two FileNodes
would stay in fuseFhMap forever.

This commit makes eraseNode again aware of different FileNodes for
a single path, makes sure the right FileNode is erased, and fixes
the memory leak.
---
 encfs/Context.cpp | 23 ++++++++++++++++-------
 encfs/Context.h   |  5 +++--
 2 files changed, 19 insertions(+), 9 deletions(-)

diff --git a/encfs/Context.cpp b/encfs/Context.cpp
index 5ba651a..5585275 100644
--- a/encfs/Context.cpp
+++ b/encfs/Context.cpp
@@ -89,7 +89,7 @@ std::shared_ptr<FileNode> EncFS_Context::lookupNode(const char *path) {
 
   FileMap::iterator it = openFiles.find(std::string(path));
   if (it != openFiles.end()) {
-    // all the items in the set point to the same node.. so just use the
+    // every entry in the list is fine... so just use the
     // first
     return it->second.front();
   }
@@ -125,16 +125,25 @@ void EncFS_Context::eraseNode(const char *path, std::shared_ptr<FileNode> fnode)
 
   FileMap::iterator it = openFiles.find(std::string(path));
   rAssert(it != openFiles.end());
+  auto &list = it->second;
 
-  auto fn = it->second.front();
+  // Find "fnode" in the list of FileNodes registered under this path.
+  auto findIter = std::find(list.begin(), list.end(), fnode);
+  rAssert(findIter != list.end());
+  list.erase(findIter);
 
-  it->second.pop_front();
+  // If no reference to "fnode" remains, drop the entry from fuseFhMap
+  // and overwrite the canary.
+  findIter = std::find(list.begin(), list.end(), fnode);
+  if (findIter == list.end()) {
+    fuseFhMap.erase(fnode->fuseFh);
+    fnode->canary = CANARY_RELEASED;
+  }
 
-  // if no more references to this file, remove the record all together
-  if (it->second.empty()) {
-    fn->canary = CANARY_RELEASED;
+  // If no FileNode is registered at this path anymore, drop the entry
+  // from openFiles.
+  if (list.empty()) {
     openFiles.erase(it);
-    fuseFhMap.erase(fn->fuseFh);
   }
 }
 
diff --git a/encfs/Context.h b/encfs/Context.h
index ff1cd0f..201da47 100644
--- a/encfs/Context.h
+++ b/encfs/Context.h
@@ -21,7 +21,8 @@
 #ifndef _Context_incl_
 #define _Context_incl_
 
-#include <forward_list>
+#include <list>
+#include <algorithm>
 #include <memory>
 #include <pthread.h>
 #include <set>
@@ -85,7 +86,7 @@ class EncFS_Context {
    */
 
   typedef std::unordered_map<std::string,
-                             std::forward_list<std::shared_ptr<FileNode>>>
+                             std::list<std::shared_ptr<FileNode>>>
       FileMap;
 
   mutable pthread_mutex_t contextMutex;