Clean up locking.

native_client_sdk/src/libraries/nacl_io/kernel_object.cc

Index: native_client_sdk/src/libraries/nacl_io/kernel_object.cc
diff --git a/native_client_sdk/src/libraries/nacl_io/kernel_object.cc b/native_client_sdk/src/libraries/nacl_io/kernel_object.cc
index f593d21ce29e0808304098164541cb7ee9622e79..14f3d2aa8484f676ae7a14ba4b15e0cff34e9148 100644
--- a/native_client_sdk/src/libraries/nacl_io/kernel_object.cc
+++ b/native_client_sdk/src/libraries/nacl_io/kernel_object.cc
@@ -23,30 +23,58 @@
 #include "sdk_util/scoped_ref.h"
 
 KernelObject::KernelObject() {
-  pthread_mutex_init(&kernel_lock_, NULL);
-  pthread_mutex_init(&process_lock_, NULL);
+  pthread_mutex_init(&mount_lock_, NULL);
+  pthread_mutex_init(&handle_lock_, NULL);
+  pthread_mutex_init(&cwd_lock_,NULL);
+
+  cwd_ = "/";
 }
 
 KernelObject::~KernelObject() {
-  pthread_mutex_destroy(&process_lock_);
-  pthread_mutex_destroy(&kernel_lock_);
+  pthread_mutex_destroy(&cwd_lock_);
+  pthread_mutex_destroy(&handle_lock_);
+  pthread_mutex_destroy(&mount_lock_);
+}
+
+Error KernelObject::MountAtPath(const ScopedMount& mnt,
+                                const std::string& path) {
+  std::string abs_targ = GetAbsPath(path);

[binji, 2013/07/10 21:24:34]

nit: how about just "abs_path"

[noelallen1, 2013/07/10 22:11:07]

Done.

+
+  AutoLock lock(&mount_lock_);
+  if (mounts_.find(abs_targ) != mounts_.end())
+    return EBUSY;
+
+  mounts_[abs_targ] = mnt;
+  return 0;
+}
+
+Error KernelObject::UnmountPath(const std::string& path) {
+  std::string abs_targ = GetAbsPath(path);

[binji, 2013/07/10 21:24:34]

ditto

[noelallen1, 2013/07/10 22:11:07]

Done.

+
+  AutoLock lock(&mount_lock_);
+  MountMap_t::iterator it = mounts_.find(abs_targ);
+  if (mounts_.end() == it)
+    return EINVAL;
+
+  // It is only legal to unmount if there are no open references
+  if (it->second->RefCount() != 1)
+    return EBUSY;
+
+  mounts_.erase(it);
+  return 0;
 }
 

[binji, 2013/07/10 21:24:34]

nit: remove extra line

[noelallen1, 2013/07/10 22:11:07]

Done.

+
 // Uses longest prefix to find the mount for the give path, then
 // acquires the mount and returns it with a relative path.
-Error KernelObject::AcquireMountAndPath(const std::string& relpath,
-                                        ScopedMount* out_mount,
-                                        Path* out_path) {
-  out_mount->reset(NULL);
+Error KernelObject::AcquireMountAndRelPath(const std::string& path,
+                                           ScopedMount* out_mount,
+                                           Path* out_path) {
+  Path abs_path(GetAbsPath(path));
 
+  AutoLock lock(&mount_lock_);

[binji, 2013/07/10 21:24:34]

This can move below clearing out_mount/out_path.

[noelallen1, 2013/07/10 22:11:07]

Done.

+  out_mount->reset(NULL);
   *out_path = Path();
-  Path abs_path;
-  {
-    AutoLock lock(&process_lock_);
-    abs_path = GetAbsPathLocked(relpath);
-  }
-
-  AutoLock lock(&kernel_lock_);
 
   // Find longest prefix
   size_t max = abs_path.Size();
@@ -64,10 +92,70 @@ Error KernelObject::AcquireMountAndPath(const std::string& relpath,
   return ENOTDIR;
 }
 
+// Given a path, acquire the associated mount and node, creating then

[binji, 2013/07/10 21:24:34]

s/then/the/

[noelallen1, 2013/07/10 22:11:07]

Done.

+// node if needed based on the provided flags.
+Error KernelObject::AcquireMountAndNode(const std::string& path,
+                                        int oflags,
+                                        ScopedMount* out_mount,
+                                        ScopedMountNode* out_node) {
+  Path rel;
+  out_mount->reset(NULL);
+  out_node->reset(NULL);
+  Error error = AcquireMountAndRelPath(path, out_mount, &rel);
+  if (error)
+    return error;
+
+  error = (*out_mount)->Open(rel, oflags, out_node);
+  if (error)
+    return error;
+
+  return 0;
+}
+
+std::string KernelObject::GetAbsPath(const std::string& path) {

[binji, 2013/07/10 21:24:34]

Why not return this is a Path, like before? Otherwise if a caller wants a Path
they'll have to re-split it.

[noelallen1, 2013/07/10 22:11:07]

Done.

+  AutoLock lock(&cwd_lock_);
+
+  Path abs_path(cwd_);
+  if (path[0] == '/') {
+    abs_path = path;
+  } else {
+    abs_path = cwd_;
+    abs_path.Append(path);
+  }
+
+  return abs_path.Join();
+}
+
+std::string KernelObject::GetCWD() {
+  AutoLock lock(&cwd_lock_);
+  std::string out = cwd_;
+
+  return out;
+}
+
+Error KernelObject::SetCWD(const std::string& path) {
+  std::string abs_path = GetAbsPath(path);
+
+  ScopedMount mnt;
+  ScopedMountNode node;
+
+  Error error = AcquireMountAndNode(abs_path, O_RDONLY, &mnt, &node);
+  if (error)
+    return error;
+
+  struct stat statbuf;

[binji, 2013/07/10 21:24:34]

unused, remove

[noelallen1, 2013/07/10 22:11:07]

Done.

+  if ((node->GetType() & S_IFDIR) == 0)
+    return ENOTDIR;
+
+  AutoLock lock(&cwd_lock_);
+  cwd_ = abs_path;
+  return 0;
+}
+
 Error KernelObject::AcquireHandle(int fd, ScopedKernelHandle* out_handle) {
   out_handle->reset(NULL);
 
-  AutoLock lock(&process_lock_);
+  AutoLock lock(&handle_lock_);
   if (fd < 0 || fd >= static_cast<int>(handle_map_.size()))
     return EBADF;
 
@@ -78,7 +166,7 @@ Error KernelObject::AcquireHandle(int fd, ScopedKernelHandle* out_handle) {
 }
 
 int KernelObject::AllocateFD(const ScopedKernelHandle& handle) {
-  AutoLock lock(&process_lock_);
+  AutoLock lock(&handle_lock_);
   int id;
 
   // If we can recycle and FD, use that first
@@ -99,7 +187,7 @@ void KernelObject::FreeAndReassignFD(int fd, const ScopedKernelHandle& handle) {
   if (NULL == handle) {
     FreeFD(fd);
   } else {
-    AutoLock lock(&process_lock_);
+    AutoLock lock(&handle_lock_);
 
     // If the required FD is larger than the current set, grow the set
     if (fd >= handle_map_.size())
@@ -110,7 +198,7 @@ void KernelObject::FreeAndReassignFD(int fd, const ScopedKernelHandle& handle) {
 }
 
 void KernelObject::FreeFD(int fd) {
-  AutoLock lock(&process_lock_);
+  AutoLock lock(&handle_lock_);
 
   handle_map_[fd].reset(NULL);
   free_fds_.push_back(fd);
@@ -118,18 +206,3 @@ void KernelObject::FreeFD(int fd) {
   // Force lower numbered FD to be available first.
   std::push_heap(free_fds_.begin(), free_fds_.end(), std::greater<int>());
 }
-
-Path KernelObject::GetAbsPathLocked(const std::string& path) {
-  // Generate absolute path
-  Path abs_path(cwd_);
-
-  if (path[0] == '/') {
-    abs_path = path;
-  } else {
-    abs_path = cwd_;
-    abs_path.Append(path);
-  }
-
-  return abs_path;
-}
-