Separate directory loading into steps to avoid blocking samba thread.

.idea/misc.xml

@@ -37,7 +37,7 @@
     <ConfirmationsSetting value="0" id="Add" />
     <ConfirmationsSetting value="0" id="Remove" />
   </component>
-  <component name="ProjectRootManager" version="2" languageLevel="JDK_1_7" default="true" assert-keyword="true" jdk-15="true" project-jdk-name="1.8" project-jdk-type="JavaSDK">
+  <component name="ProjectRootManager" version="2" languageLevel="JDK_1_8" default="true" assert-keyword="true" jdk-15="true" project-jdk-name="1.8" project-jdk-type="JavaSDK">
     <output url="file://$PROJECT_DIR$/build/classes" />
   </component>
   <component name="ProjectType">

app/build.gradle

@@ -2,7 +2,7 @@ apply plugin: 'com.android.application'
 
 android {
     compileSdkVersion 26
-    buildToolsVersion "25.0.2"
+    buildToolsVersion "26.0.0"
     defaultConfig {
         applicationId "com.google.android.sambadocumentsprovider"
         minSdkVersion 21
@@ -45,7 +45,7 @@ dependencies {
     androidTestCompile('com.android.support.test.espresso:espresso-core:2.2.2', {
         exclude group: 'com.android.support', module: 'support-annotations'
     })
-    compile 'com.android.support:appcompat-v7:25.1.0'
+    compile 'com.android.support:appcompat-v7:25.4.0'
-    compile 'com.android.support:design:25.1.0'
+    compile 'com.android.support:design:25.4.0'
     testCompile 'junit:junit:4.12'
 }

app/src/main/cpp/jni_helper/JniHelper.cc

@@ -97,40 +97,6 @@ create_directory_entry(JNIEnv* env, const struct smbc_dirent &ent) {
   return entry;
 }
 
-static jobject create_array_list(JNIEnv* env) {
-  static const jclass arrayListClass =
-      classCache_.get(env, "java/util/ArrayList");
-  static const jmethodID arrayListConstructor =
-      env->GetMethodID(arrayListClass, "<init>", "()V");
-
-  return env->NewObject(arrayListClass, arrayListConstructor);
-}
-
-static void
-add_object_to_array_list(JNIEnv *env, jobject arrayList, jobject obj) {
-  static const jclass arrayListClass =
-      classCache_.get(env, "java/util/ArrayList");
-  static const jmethodID addMethod =
-      env->GetMethodID(arrayListClass, "add", "(Ljava/lang/Object;)Z");
-
-  env->CallBooleanMethod(arrayList, addMethod, obj);
-}
-
-static int add_dir_entry_to_array_list(
-    JniContext<jobject> context, struct smbc_dirent* dirent) {
-  jobject entry = create_directory_entry(context.env, *dirent);
-  if (entry == NULL) {
-    return -1;
-  }
-  add_object_to_array_list(context.env, context.instance, entry);
-  if (context.env->ExceptionCheck()) {
-    return -1;
-  }
-  // We're done with this entry, remove local ref to avoid leak.
-  context.env->DeleteLocalRef(entry);
-  return 0;
-}
-
 static void
 throw_new_file_not_found_exception(JNIEnv *env, const char *fmt, ...) {
   char message[256];
@@ -184,36 +150,20 @@ throw_new_auth_failed_exception(JNIEnv* env) {
   env->Throw(authFailedException);
 }
 
-jobject
+jint
-Java_com_google_android_sambadocumentsprovider_nativefacade_NativeSambaFacade_readDir(
+Java_com_google_android_sambadocumentsprovider_nativefacade_NativeSambaFacade_openDir(
     JNIEnv *env, jobject instance, jlong pointer, jstring uri_) {
   const char *uri = env->GetStringUTFChars(uri_, 0);
   if (uri == NULL) {
-    return NULL;
+    return -1;
   }
 
-  jobject arrayList = create_array_list(env);
-  if (arrayList == NULL) {
-    // Java exception happened.
-    env->ReleaseStringUTFChars(uri_, uri);
-    return NULL;
-  }
-
-  JniContext<jobject> context(env, arrayList);
   SambaClient::SambaClient *client =
       reinterpret_cast<SambaClient::SambaClient *>(pointer);
-  int result = client->ReadDir(
+  int fd = client->OpenDir(uri);
-      uri,
-      JniCallback<jobject, struct smbc_dirent*>(
-          context, add_dir_entry_to_array_list));
 
-  if (env->ExceptionCheck()) {
+  if (fd < 0) {
-    // Java exception happened.
+    int err = -fd;
-    goto bail;
-  }
-
-  if (result < 0) {
-    int err = -result;
     switch (err) {
       case ENODEV:
       case ENOENT:
@@ -226,14 +176,13 @@ Java_com_google_android_sambadocumentsprovider_nativefacade_NativeSambaFacade_re
         throw_new_auth_failed_exception(env);
         break;
       default:
-        throw_new_errno_exception(env, "readDir", err);
+        throw_new_errno_exception(env, "openDir", err);
     }
   }
 
-  bail:
   env->ReleaseStringUTFChars(uri_, uri);
 
-  return arrayList;
+  return fd;
 }
 
 static jobject create_structstat(JNIEnv *env, const struct stat &st) {
@@ -527,21 +476,52 @@ Java_com_google_android_sambadocumentsprovider_nativefacade_NativeSambaFacade_op
   return fd;
 }
 
+jobject Java_com_google_android_sambadocumentsprovider_nativefacade_SambaDir_readDir(
+    JNIEnv *env, jobject instance, jlong pointer, jint dh) {
+  SambaClient::SambaClient *client =
+      reinterpret_cast<SambaClient::SambaClient*>(pointer);
+
+  const struct smbc_dirent* dirent;
+
+  int result = client->ReadDir(dh, &dirent);
+  if (result < 0) {
+    throw_new_errno_exception(env, "readDir", -result);
+    return NULL;
+  }
+
+  if (dirent == NULL) {
+    // This is a normal case, indicating that we finished reading this directory.
+    return NULL;
+  }
+
+  return create_directory_entry(env, *dirent);
+}
+
+void Java_com_google_android_sambadocumentsprovider_nativefacade_SambaDir_close(
+    JNIEnv *env, jobject instance, jlong pointer, jint dh) {
+  SambaClient::SambaClient *client =
+      reinterpret_cast<SambaClient::SambaClient*>(pointer);
+
+  int result = client->CloseDir(dh);
+  if (result < 0) {
+    throw_new_errno_exception(env, "close", -result);
+  }
+}
+
 jobject Java_com_google_android_sambadocumentsprovider_nativefacade_SambaFile_fstat(
         JNIEnv *env,
         jobject instance,
         jlong pointer,
         jint fd) {
   SambaClient::SambaClient *client =
-          reinterpret_cast<SambaClient::SambaClient*>(pointer);
+      reinterpret_cast<SambaClient::SambaClient*>(pointer);
-
-  jobject stat = NULL;
 
   struct stat st;
 
   int result = client->Fstat(fd, &st);
   if (result < 0) {
     throw_new_errno_exception(env, "stat", -result);
+    return NULL;
   }
 
   return create_structstat(env, st);
@@ -555,7 +535,7 @@ jlong Java_com_google_android_sambadocumentsprovider_nativefacade_SambaFile_seek
     jlong offset,
     jint whence) {
   SambaClient::SambaClient *client =
-          reinterpret_cast<SambaClient::SambaClient*>(pointer);
+      reinterpret_cast<SambaClient::SambaClient*>(pointer);
 
   jlong result = client->SeekFile(fd, offset, whence);
   if (result < 0) {
@@ -565,8 +545,6 @@ jlong Java_com_google_android_sambadocumentsprovider_nativefacade_SambaFile_seek
   return result;
 }
 
-
-
 jlong Java_com_google_android_sambadocumentsprovider_nativefacade_SambaFile_read(
     JNIEnv *env,
     jobject instance,

app/src/main/cpp/jni_helper/JniHelper.h

@@ -32,8 +32,8 @@ JNIEXPORT void JNICALL
     Java_com_google_android_sambadocumentsprovider_nativefacade_NativeSambaFacade_nativeDestroy(
     JNIEnv *env, jobject instance, jlong pointer);
 
-JNIEXPORT jobject JNICALL
+JNIEXPORT jint JNICALL
-    Java_com_google_android_sambadocumentsprovider_nativefacade_NativeSambaFacade_readDir(
+    Java_com_google_android_sambadocumentsprovider_nativefacade_NativeSambaFacade_openDir(
     JNIEnv *env, jobject instance, jlong pointer, jstring uri_);
 
 JNIEXPORT jobject JNICALL
@@ -64,6 +64,14 @@ JNIEXPORT jint JNICALL
     Java_com_google_android_sambadocumentsprovider_nativefacade_NativeSambaFacade_openFile(
     JNIEnv *env, jobject instance, jlong pointer, jstring uri_, jstring mode_);
 
+JNIEXPORT jobject JNICALL
+    Java_com_google_android_sambadocumentsprovider_nativefacade_SambaDir_readDir(
+    JNIEnv *env, jobject instance, jlong pointer, jint fd);
+
+JNIEXPORT void JNICALL
+    Java_com_google_android_sambadocumentsprovider_nativefacade_SambaDir_close(
+    JNIEnv *env, jobject instance, jlong pointer, jint fd);
+
 JNIEXPORT jlong JNICALL
     Java_com_google_android_sambadocumentsprovider_nativefacade_SambaFile_read(
     JNIEnv *env, jobject instance, jlong pointer, jint fd, jobject buffer, jint maxlen);
@@ -74,7 +82,7 @@ JNIEXPORT jlong JNICALL
 
 JNIEXPORT jobject JNICALL
     Java_com_google_android_sambadocumentsprovider_nativefacade_SambaFile_fstat(
-        JNIEnv *env, jobject instance, jlong pointer, jint fd);
+    JNIEnv *env, jobject instance, jlong pointer, jint fd);
 
 JNIEXPORT jlong JNICALL
     Java_com_google_android_sambadocumentsprovider_nativefacade_SambaFile_write(

app/src/main/cpp/samba_client/SambaClient.cc

@@ -130,31 +130,40 @@ static const char* getTypeName(unsigned int smbc_type) {
 }
 
 int
-SambaClient::ReadDir(
+SambaClient::OpenDir(const char *url) {
-    const char *url, const Callback<struct smbc_dirent*> &entryHandler) {
+  LOGD(TAG, "Opening dir at %s.", url);
-  LOGD(TAG, "Reading dir at %s.", url);
+  const int fd = smbc_opendir(url);
-  const int dir = smbc_opendir(url);
+  if (fd < 0) {
-  if (dir < 0) {
     int err = errno;
     LOGE(TAG, "Failed to open dir at %s. Errno: %x", url, err);
     return -err;
   }
 
-  struct smbc_dirent *dirent = NULL;
+  return fd;
-  while ((dirent = smbc_readdir(dir)) != NULL) {
+}
-    LOGV(TAG, "Found entry name: %s, comment: %s, type: %s.",
+
-         dirent->name, dirent->comment, getTypeName(dirent->smbc_type));
+int
-    if (entryHandler(dirent) < 0) {
+SambaClient::ReadDir(const int dh, const struct smbc_dirent ** dirent) {
-      // Java exceptions.
+  LOGD(TAG, "Reading dir for %x.", dh);
-      smbc_closedir(dir);
+  *dirent = smbc_readdir(dh);
-      return -1;
+  if (*dirent == NULL) {
-    }
+    LOGV(TAG, "Finished reading dir ent for %x.", dh);
-  }
+  } else {
+    LOGV(TAG, "Found entry name: %s, comment: %s, type: %s.",
+         (*dirent)->name, (*dirent)->comment, getTypeName((*dirent)->smbc_type));
+  }
+  return 0;
+}
+
+int
+SambaClient::CloseDir(const int dh) {
+  LOGD(TAG, "Close dir for %x.", dh);
+  const int ret = smbc_closedir(dh);
 
-  const int ret = smbc_closedir(dir);
   if (ret) {
     int err = errno;
-    LOGW(TAG, "Failed to close dir %d at %s. Errno: %x.", dir, url, err);
+    LOGW(TAG, "Failed to close dir with dh %x. Errno: %x.", dh, err);
+    return -err;
   }
 
   return 0;

app/src/main/cpp/samba_client/SambaClient.h

@@ -35,7 +35,11 @@ class SambaClient {
 
   bool Init(const bool debug, const CredentialCache *credentialCache);
 
-  int ReadDir(const char *url, const Callback<struct ::smbc_dirent*> &entryHandler);
+  int OpenDir(const char *url);
+
+  int ReadDir(const int dh, const struct smbc_dirent** dirent);
+
+  int CloseDir(const int dh);
 
   int Stat(const char *url, struct stat *st);
 

app/src/main/java/com/google/android/sambadocumentsprovider/document/DocumentMetadata.java

@@ -27,6 +27,7 @@ import android.util.Log;
 import android.webkit.MimeTypeMap;
 import com.google.android.sambadocumentsprovider.base.DirectoryEntry;
 import com.google.android.sambadocumentsprovider.nativefacade.SmbClient;
+import com.google.android.sambadocumentsprovider.nativefacade.SmbDir;
 import java.io.IOException;
 import java.util.HashMap;
 import java.util.List;
@@ -203,11 +204,11 @@ public class DocumentMetadata {
   }
 
   public void loadChildren(SmbClient client) throws IOException {
-    try {
+    try (final SmbDir dir = client.openDir(mUri.toString())) {
-      List<DirectoryEntry> entries = client.readDir(mUri.toString());
 
-      Map<Uri, DocumentMetadata> children = new HashMap<>(entries.size());
+      Map<Uri, DocumentMetadata> children = new HashMap<>();
-      for (DirectoryEntry entry : entries) {
+      DirectoryEntry entry;
+      while ((entry = dir.readDir()) != null) {
         Uri childUri = DocumentMetadata.buildChildUri(mUri, entry);
         if (childUri != null) {
           children.put(childUri, new DocumentMetadata(childUri, entry));

app/src/main/java/com/google/android/sambadocumentsprovider/nativefacade/MessageValues.java

@@ -34,6 +34,7 @@ class MessageValues<T> implements AutoCloseable {
 
   private volatile T mObj;
   private volatile int mInt;
+  private volatile long mLong;
   private volatile IOException mException;
   private volatile RuntimeException mRuntimeException;
 
@@ -62,6 +63,15 @@ class MessageValues<T> implements AutoCloseable {
     return mInt;
   }
 
+  long getLong() throws IOException {
+    checkException();
+    return mLong;
+  }
+
+  void setLong(long value) {
+    mLong = value;
+  }
+
   void setInt(int value) {
     mInt = value;
   }
@@ -87,6 +97,7 @@ class MessageValues<T> implements AutoCloseable {
   public void close() {
     mObj = null;
     mInt = 0;
+    mLong = 0L;
     mException = null;
     mRuntimeException = null;
     POOL.release(this);

app/src/main/java/com/google/android/sambadocumentsprovider/nativefacade/NativeSambaFacade.java

@@ -58,10 +58,10 @@ class NativeSambaFacade implements SmbClient {
   }
 
   @Override
-  public List<DirectoryEntry> readDir(String uri) throws IOException {
+  public SmbDir openDir(String uri) throws IOException {
     try {
       checkNativeHandler();
-      return readDir(mNativeHandler, uri);
+      return new SambaDir(mNativeHandler, openDir(mNativeHandler, uri));
     } catch (ErrnoException e) {
       throw new IOException("Failed to read directory " + uri, e);
     }
@@ -147,7 +147,7 @@ class NativeSambaFacade implements SmbClient {
 
   private native void nativeDestroy(long handler);
 
-  private native List<DirectoryEntry> readDir(long handler, String uri) throws ErrnoException;
+  private native int openDir(long handler, String uri) throws ErrnoException;
 
   private native StructStat stat(long handler, String uri) throws ErrnoException;
 

app/src/main/java/com/google/android/sambadocumentsprovider/nativefacade/SambaDir.java

@@ -0,0 +1,58 @@
+/*
+ * Copyright 2017 Google Inc.
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+package com.google.android.sambadocumentsprovider.nativefacade;
+
+import android.support.annotation.Nullable;
+import android.system.ErrnoException;
+import com.google.android.sambadocumentsprovider.base.DirectoryEntry;
+import java.io.IOException;
+
+class SambaDir implements SmbDir {
+
+  private final long mNativeHandler;
+  private int mNativeDh;
+
+  SambaDir(long nativeHandler, int nativeFd) {
+    mNativeHandler = nativeHandler;
+    mNativeDh = nativeFd;
+  }
+
+  @Override
+  public DirectoryEntry readDir() throws IOException {
+    try {
+      return readDir(mNativeHandler, mNativeDh);
+    } catch (ErrnoException e) {
+      throw new IOException(e);
+    }
+  }
+
+
+  @Override
+  public void close() throws IOException {
+    try {
+      int dh = mNativeDh;
+      mNativeDh = -1;
+      close(mNativeHandler, dh);
+    } catch (ErrnoException e) {
+      throw new IOException(e);
+    }
+  }
+
+  private native @Nullable DirectoryEntry readDir(long handler, int fd) throws ErrnoException;
+  private native void close(long handler, int fd) throws ErrnoException;
+}

app/src/main/java/com/google/android/sambadocumentsprovider/nativefacade/SambaDirClient.java

@@ -0,0 +1,93 @@
+/*
+ * Copyright 2017 Google Inc.
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+package com.google.android.sambadocumentsprovider.nativefacade;
+
+import android.os.Handler;
+import android.os.Looper;
+import android.os.Message;
+import android.support.annotation.IntDef;
+import android.support.annotation.Nullable;
+import com.google.android.sambadocumentsprovider.base.DirectoryEntry;
+import java.io.IOException;
+import java.lang.annotation.Retention;
+import java.lang.annotation.RetentionPolicy;
+
+class SambaDirClient extends BaseClient implements SmbDir {
+
+  @IntDef({ READ_DIR, CLOSE })
+  @Retention(RetentionPolicy.SOURCE)
+  @interface Operation {}
+  private static final int READ_DIR = 0;
+  private static final int CLOSE = READ_DIR + 1;
+
+  SambaDirClient(Looper looper, SmbDir smbDirImpl) {
+    mHandler = new SambaDirHandler(looper, smbDirImpl);
+  }
+
+  @Nullable
+  @Override
+  public DirectoryEntry readDir() throws IOException {
+    try (MessageValues<DirectoryEntry> messageValues = MessageValues.obtain()) {
+      final Message msg = mHandler.obtainMessage(READ_DIR, messageValues);
+      enqueue(msg);
+      return messageValues.getObj();
+    }
+  }
+
+  @Override
+  public void close() throws IOException {
+    try (MessageValues<?> messageValues = MessageValues.obtain()) {
+      final Message msg = mHandler.obtainMessage(CLOSE, messageValues);
+      enqueue(msg);
+      messageValues.checkException();
+    }
+  }
+
+  private static class SambaDirHandler extends BaseHandler {
+
+    private final SmbDir mSmbDirImpl;
+
+    private SambaDirHandler(Looper looper, SmbDir smbDirImpl) {
+      super(looper);
+
+      mSmbDirImpl = smbDirImpl;
+    }
+
+    @Override
+    @SuppressWarnings("unchecked")
+    public void processMessage(Message msg) {
+      final MessageValues<DirectoryEntry> messageValues = (MessageValues<DirectoryEntry>) msg.obj;
+      try {
+        switch (msg.what) {
+          case READ_DIR:
+            messageValues.setObj(mSmbDirImpl.readDir());
+            break;
+          case CLOSE:
+            mSmbDirImpl.close();
+            break;
+          default:
+            throw new UnsupportedOperationException("Unknown operation " + msg.what);
+        }
+      } catch (RuntimeException e) {
+        messageValues.setRuntimeException(e);
+      } catch (IOException e) {
+        messageValues.setException(e);
+      }
+    }
+  }
+}

app/src/main/java/com/google/android/sambadocumentsprovider/nativefacade/SambaFacadeClient.java

@@ -77,8 +77,8 @@ class SambaFacadeClient extends BaseClient implements SmbFacade {
   }
 
   @Override
-  public List<DirectoryEntry> readDir(String uri) throws IOException {
+  public SmbDir openDir(String uri) throws IOException {
-    try (final MessageValues<List<DirectoryEntry>> messageValues = MessageValues.obtain()) {
+    try (final MessageValues<SmbDir> messageValues = MessageValues.obtain()) {
       final Message msg = obtainMessage(READ_DIR, messageValues, uri);
       enqueue(msg);
       return messageValues.getObj();
@@ -195,7 +195,7 @@ class SambaFacadeClient extends BaseClient implements SmbFacade {
             mClientImpl.reset();
             break;
           case READ_DIR:
-            messageValues.setObj(mClientImpl.readDir(uri));
+            messageValues.setObj(mClientImpl.openDir(uri));
             break;
           case STAT:
             messageValues.setObj(mClientImpl.stat(uri));

app/src/main/java/com/google/android/sambadocumentsprovider/nativefacade/SmbClient.java

@@ -26,7 +26,7 @@ public interface SmbClient {
 
   void reset();
 
-  List<DirectoryEntry> readDir(String uri) throws IOException;
+  SmbDir openDir(String uri) throws IOException;
 
   StructStat stat(String uri) throws IOException;
 

app/src/main/java/com/google/android/sambadocumentsprovider/nativefacade/SmbDir.java

@@ -0,0 +1,29 @@
+/*
+ * Copyright 2017 Google Inc.
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+package com.google.android.sambadocumentsprovider.nativefacade;
+
+import android.support.annotation.Nullable;
+import com.google.android.sambadocumentsprovider.base.DirectoryEntry;
+import java.io.Closeable;
+import java.io.IOException;
+
+public interface SmbDir extends Closeable {
+
+  @Nullable DirectoryEntry readDir() throws IOException;
+
+}

build.gradle

@@ -12,6 +12,9 @@ buildscript {
 allprojects {
     repositories {
         jcenter()
+        maven {
+            url "https://maven.google.com"
+        }
     }
 }