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

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

removed the awareness for different FileNodes.

The crashes have been fixed by

  e2f0f8e3c6
  "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.
This commit is contained in:
Jakob Unterwurzacher 2017-07-24 22:34:01 +02:00
parent e2f0f8e3c6
commit 9ed8d704dd
2 changed files with 19 additions and 9 deletions

View File

@ -89,7 +89,7 @@ std::shared_ptr<FileNode> EncFS_Context::lookupNode(const char *path) {
FileMap::iterator it = openFiles.find(std::string(path)); FileMap::iterator it = openFiles.find(std::string(path));
if (it != openFiles.end()) { 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 // first
return it->second.front(); 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)); FileMap::iterator it = openFiles.find(std::string(path));
rAssert(it != openFiles.end()); 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 no FileNode is registered at this path anymore, drop the entry
if (it->second.empty()) { // from openFiles.
fn->canary = CANARY_RELEASED; if (list.empty()) {
openFiles.erase(it); openFiles.erase(it);
fuseFhMap.erase(fn->fuseFh);
} }
} }

View File

@ -21,7 +21,8 @@
#ifndef _Context_incl_ #ifndef _Context_incl_
#define _Context_incl_ #define _Context_incl_
#include <forward_list> #include <list>
#include <algorithm>
#include <memory> #include <memory>
#include <pthread.h> #include <pthread.h>
#include <set> #include <set>
@ -85,7 +86,7 @@ class EncFS_Context {
*/ */
typedef std::unordered_map<std::string, typedef std::unordered_map<std::string,
std::forward_list<std::shared_ptr<FileNode>>> std::list<std::shared_ptr<FileNode>>>
FileMap; FileMap;
mutable pthread_mutex_t contextMutex; mutable pthread_mutex_t contextMutex;