Fixed issue #9

Now storing the working directory for each Process in the FileSystem object instead of relying on uv_chdir and uv_cwd. Added test case.
LiquidPlayer · Jan 9, 2017 · 9d14ea0 · 9d14ea0
1 parent 97e8ef5
commit 9d14ea0
Show file tree

Hide file tree

Showing 11 changed files with 148 additions and 116 deletions.
diff --git a/.idea/encodings.xml b/.idea/encodings.xml
diff --git a/.idea/gradle.xml b/.idea/gradle.xml
diff --git a/.idea/inspectionProfiles/Project_Default.xml b/.idea/inspectionProfiles/Project_Default.xml
diff --git a/.idea/inspectionProfiles/profiles_settings.xml b/.idea/inspectionProfiles/profiles_settings.xml
diff --git a/.idea/misc.xml b/.idea/misc.xml
diff --git a/.idea/vcs.xml b/.idea/vcs.xml
diff --git a/LiquidCoreAndroid/jni/node/NodeInstance.cpp b/LiquidCoreAndroid/jni/node/NodeInstance.cpp
@@ -228,10 +228,14 @@ void NodeInstance::Chdir(const FunctionCallbackInfo<Value>& args) {
 
   BufferValue path(args.GetIsolate(), nodedroid::fs_(env, args[0], _FS_ACCESS_RD));
 
+  // Only using uv_chdir to validate path -- the default path should never be used
   int err = uv_chdir(*path);
   if (err) {
     return env->ThrowUVException(err, "uv_chdir");
   }
+
+  // The real default path is held in the filesystem object
+  nodedroid::chdir_(env, args[0]);
 }
 
 std::map<Environment*,NodeInstance*> NodeInstance::instance_map;
@@ -253,19 +257,24 @@ void NodeInstance::Exit(const FunctionCallbackInfo<Value>& args) {
 
 void NodeInstance::Cwd(const FunctionCallbackInfo<Value>& args) {
   Environment* env = Environment::GetCurrent(args);
-  char buf[PATH_MAX];
+  Local<Value> aliased;
 
-  size_t cwd_len = sizeof(buf);
-  int err = uv_cwd(buf, &cwd_len);
-  if (err) {
-    return env->ThrowUVException(err, "uv_cwd");
-  }
+  aliased = nodedroid::cwd_(env);
+  if (aliased->IsUndefined()) {
+      char buf[PATH_MAX];
 
-  Local<String> cwd = String::NewFromUtf8(env->isolate(),
-                                          buf,
-                                          String::kNormalString,
-                                          cwd_len);
-  Local<Value> aliased = nodedroid::alias_(env, cwd);
+      size_t cwd_len = sizeof(buf);
+      int err = uv_cwd(buf, &cwd_len);
+      if (err) {
+        return env->ThrowUVException(err, "uv_cwd");
+      }
+
+      Local<String> cwd = String::NewFromUtf8(env->isolate(),
+                                              buf,
+                                              String::kNormalString,
+                                              cwd_len);
+      aliased = nodedroid::alias_(env, cwd);
+  }
   args.GetReturnValue().Set(aliased);
 }
 

diff --git a/LiquidCoreAndroid/jni/node/nodedroid_file.cc b/LiquidCoreAndroid/jni/node/nodedroid_file.cc
@@ -519,6 +519,44 @@ Local<Value> alias_(Environment *env, Local<Value> path)
     return path;
 }
 
+Local<Value> chdir_(Environment *env, Local<Value> path)
+{
+    HandleScope handle_scope(env->isolate());
+    Context::Scope context_scope(env->context());
+
+    Local<v8::Private> privateKey = v8::Private::ForApi(env->isolate(),
+        String::NewFromUtf8(env->isolate(), "__fs"));
+    Local<Object> globalObj = env->context()->Global();
+    Maybe<bool> result = globalObj->HasPrivate(env->context(), privateKey);
+    if (result.IsJust() && result.FromJust()) {
+      Local<Value> fsVal;
+      if(globalObj->GetPrivate(env->context(), privateKey).ToLocal(&fsVal)) {
+        Local<Object> fsObj = fsVal->ToObject(env->context()).ToLocalChecked();
+        fsObj->Set(env->context(), String::NewFromUtf8(env->isolate(), "cwd"), path);
+      }
+    }
+    return path;
+}
+
+Local<Value> cwd_(Environment *env)
+{
+    HandleScope handle_scope(env->isolate());
+    Context::Scope context_scope(env->context());
+
+    Local<v8::Private> privateKey = v8::Private::ForApi(env->isolate(),
+        String::NewFromUtf8(env->isolate(), "__fs"));
+    Local<Object> globalObj = env->context()->Global();
+    Maybe<bool> result = globalObj->HasPrivate(env->context(), privateKey);
+    if (result.IsJust() && result.FromJust()) {
+      Local<Value> fsVal;
+      if(globalObj->GetPrivate(env->context(), privateKey).ToLocal(&fsVal)) {
+        Local<Object> fsObj = fsVal->ToObject(env->context()).ToLocalChecked();
+        return fsObj->Get(env->context(), String::NewFromUtf8(env->isolate(), "cwd")).ToLocalChecked();
+      }
+    }
+    return Local<Value>::New(env->isolate(),Undefined(env->isolate()));
+}
+
 static void Access(const FunctionCallbackInfo<Value>& args) {
   Environment* env = Environment::GetCurrent(args);
   HandleScope scope(env->isolate());

diff --git a/LiquidCoreAndroid/jni/node/nodedroid_file.h b/LiquidCoreAndroid/jni/node/nodedroid_file.h
@@ -48,6 +48,8 @@ namespace nodedroid {
 void InitFs(v8::Local<v8::Object> target);
 v8::Local<v8::Value> alias_(node::Environment *env, v8::Local<v8::Value> path);
 v8::Local<v8::Value> fs_(node::Environment *env, v8::Local<v8::Value> path, int req_access);
+v8::Local<v8::Value> chdir_(node::Environment *env, v8::Local<v8::Value> path);
+v8::Local<v8::Value> cwd_(node::Environment *env);
 
 extern "C" node::node_module fs_module;
 

diff --git a/LiquidCoreAndroid/src/androidTest/java/org/liquidplayer/node/FSTest.java b/LiquidCoreAndroid/src/androidTest/java/org/liquidplayer/node/FSTest.java
@@ -39,6 +39,7 @@ OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
 import org.liquidplayer.javascript.JSBaseArray;
 import org.liquidplayer.javascript.JSContext;
 import org.liquidplayer.javascript.JSObject;
+import org.liquidplayer.javascript.JSValue;
 
 import java.io.File;
 import java.io.InputStream;
@@ -186,4 +187,70 @@ public void onDone(JSContext ctx) {
 
     }
 
+    /**
+     * https://github.com/LiquidPlayer/LiquidCore/issues/9
+     */
+    @Test
+    public void testChdirCwdMultipleProcesses() throws Exception {
+        final String dir1 = "/home/external/persistent";
+        final String dir2 = "/home/external/cache";
+
+        final String proc1 = "process.chdir('" + dir1 + "');";
+        final String proc2 = "process.chdir('" + dir2 + "');";
+
+        final Semaphore s1 = new Semaphore(0);
+        final Semaphore s2 = new Semaphore(0);
+
+        class Contexts {
+            JSContext c1;
+            JSContext c2;
+        }
+
+        final Contexts contexts = new Contexts();
+
+        Process p1 = new Process(InstrumentationRegistry.getContext(), "_",
+                Process.kMediaAccessPermissionsRW, new Process.EventListener() {
+            @Override
+            public void onProcessStart(Process process, JSContext context) {
+                contexts.c1 = context;
+                process.keepAlive();
+                s1.release();
+            }
+
+            @Override public void onProcessAboutToExit(Process process, int exitCode) {}
+            @Override public void onProcessExit(Process process, int exitCode) {}
+            @Override public void onProcessFailed(Process process, Exception error) {}
+        });
+
+        Process p2 = new Process(InstrumentationRegistry.getContext(), "_",
+                Process.kMediaAccessPermissionsRW, new Process.EventListener() {
+            @Override
+            public void onProcessStart(Process process, JSContext context) {
+                contexts.c2 = context;
+                process.keepAlive();
+                s2.release();
+            }
+
+            @Override public void onProcessAboutToExit(Process process, int exitCode) {}
+            @Override public void onProcessExit(Process process, int exitCode) {}
+            @Override public void onProcessFailed(Process process, Exception error) {}
+        });
+
+        s1.acquire();
+        s2.acquire();
+
+        contexts.c1.evaluateScript(proc1);
+        JSValue v = contexts.c1.evaluateScript("process.cwd()");
+        assertEquals(dir1, v.toString());
+
+        contexts.c2.evaluateScript(proc2);
+        v = contexts.c2.evaluateScript("process.cwd()");
+        assertEquals(dir2, v.toString());
+
+        v = contexts.c1.evaluateScript("process.cwd()");
+        assertEquals(dir1, v.toString());
+
+        p1.letDie();
+        p2.letDie();
+   }
 }
diff --git a/LiquidCoreAndroid/src/main/java/org/liquidplayer/node/Process.java b/LiquidCoreAndroid/src/main/java/org/liquidplayer/node/Process.java
@@ -392,16 +392,18 @@ public void finalize() throws Throwable {
      * following structure
      *
      * /
-     * /cache
      * /home
-     * /external
-     *        /cache
-     *        /home
-     * /media [if permissions set]
-     *        /Pictures
-     *        /Movies
-     *        /Downloads
-     *        /... (android standard)
+     * /home/persistent
+     * /home/cache
+     * /home/external
+     * /home/external/persistent
+     * /home/external/cache
+     * /home/media
+     * /home/media/Pictures
+     * /home/media/Movies
+     * /home/media/Ringtones
+     * /home/media/Downloads
+     * /home/media/...
      *
      * Everything else will result in a ENOACES (access denied) error
      */
@@ -419,6 +421,9 @@ private class FileSystem extends JSObject {
         @jsexport(attributes = JSPropertyAttributeReadOnly)
         private Property<JSFunction> alias;
 
+        @jsexport
+        private Property<String> cwd;
+
         private String realDir(String dir) {
             return getContext().evaluateScript(
                     "(function(){return require('fs').realpathSync('" + dir + "');})()"
@@ -505,6 +510,8 @@ private void setUp() {
                 symlink(media, home + "/media");
                 aliases_.get().property("/home/media", media);
             }
+
+            cwd.set("/home");
         }
 
         private void tearDown() {
@@ -542,6 +549,7 @@ private void tearDown() {
             */
 
             fs.set(new JSFunction(ctx, "fs", ""+
+                    "if (!file.startsWith('/')) { file = this.cwd+'/'+file; }" +
                     "try { file = require('path').resolve(file); } catch (e) {}"+
                     "var access = 0;"+
                     "for (var p in this.aliases_) {"+