Failing filename completion in large NFS-mounted directories

Failing filename completion in large NFS-mounted directories

[Ben North]

I have been having a problem whereby filename completion in a large
directory which lives over a slow NFS link sometimes fails to complete
filenames which are not near the top of the directory (according to "ls
-U").

The machine exhibiting this problem gives

% uname -a
SunOS lukekelly 5.8 Generic_108528-15 sun4u sparc SUNW,Sun-Fire-280R Solaris

I Google'd and came across Stefan Monnier's email:

   http://lists.gnu.org/archive/html/emacs-devel/2005-08/msg00643.html

including this patch to dired.c:


--- dired.c     11 aoû 2005 05:27:35 -0400      1.117
+++ dired.c     14 aoû 2005 02:44:01 -0400      
@@ -224,7 +224,7 @@
 
 #ifdef EAGAIN
       if (dp == NULL && errno == EAGAIN)
-       continue;
+       { QUIT; continue; }
 #endif
 
       if (dp == NULL)
@@ -533,6 +533,10 @@
          dp = (*readfunc) (d);
 #else
          dp = readdir (d);
+#endif
+#ifdef EAGAIN
+         if (dp == NULL && errno == EAGAIN)
+           { QUIT; continue; }
 #endif
          if (!dp) break;


and tried it, but it didn't help.  truss suggested that the getdents64()
system call was being interrupted by SIGALRM, and putting some printf()s
into dired.c revealed that readdir() sometimes returned NULL with errno
set to EINTR rather than EAGAIN.  I modified Stefan's test so that it's

          if (dp == NULL && (errno == EAGAIN || errno == EINTR))

rather than just

          if (dp == NULL && errno == EAGAIN)

in both places, and this seems to have fixed my problem.  I also added
an assignment "errno = 0;" into file_name_completion() to match what's
done in directory_files_internal() and what the Solaris doc for
readdir() suggests.

Might this be useful more generally?  I'm afraid I don't have access to
very many systems so I don't know which systems return EAGAIN and which
EINTR (the system call itself returns ERESTART so perhaps that should be
checked for too?).  I guess this could become hairy but if there's a way
to do this portably it might fix this problem for all users.

Thanks,

Ben.

[I originally emailed Stefan directly and he suggested I passed it on to
the list.]

Re: Failing filename completion in large NFS-mounted directories

[Stefan Monnier]

> I Google'd and came across Stefan Monnier's email:
>    http://lists.gnu.org/archive/html/emacs-devel/2005-08/msg00643.html
> including this patch to dired.c:

Somehow I failed to install that patch.  Huh!

Seeing how mobody else reacted, I suggest the patch below, which also fixes
an ugly bug on 64bit systems.


-- Stefan

        * dired.c (directory_files_internal_unwind): Use a proper pointer
        rather than a pair of 16bit integers to store the DIR*.
        (directory_files_internal, file_name_completion): Update use.
        Handle EAGAIN and EINTR consistently, and check QUIT.


--- dired.c	22 aoû 2005 10:24:18 -0400	1.117
+++ dired.c	01 sep 2005 16:53:06 -0400	
@@ -131,7 +131,7 @@
 directory_files_internal_unwind (dh)
      Lisp_Object dh;
 {
-  DIR *d = (DIR *) ((XINT (XCAR (dh)) << 16) + XINT (XCDR (dh)));
+  DIR *d = (DIR *) XSAVE_VALUE (dh)->pointer;
   closedir (d);
   return Qnil;
 }
@@ -155,7 +155,6 @@
   int count = SPECPDL_INDEX ();
   struct gcpro gcpro1, gcpro2, gcpro3, gcpro4, gcpro5;
   DIRENTRY *dp;
-  int retry_p;
 
   /* Because of file name handlers, these functions might call
      Ffuncall, and cause a GC.  */
@@ -189,12 +188,6 @@
   /* Now *bufp is the compiled form of MATCH; don't call anything
      which might compile a new regexp until we're done with the loop!  */
 
-  /* Do this opendir after anything which might signal an error; if
-     an error is signaled while the directory stream is open, we
-     have to make sure it gets closed, and setting up an
-     unwind_protect to do so would be a pain.  */
- retry:
-
   d = opendir (SDATA (dirfilename));
   if (d == NULL)
     report_file_error ("Opening directory", Fcons (directory, Qnil));
@@ -203,8 +196,7 @@
      file-attributes on filenames, both of which can throw, so we must
      do a proper unwind-protect.  */
   record_unwind_protect (directory_files_internal_unwind,
-			 Fcons (make_number (((unsigned long) d) >> 16),
-				make_number (((unsigned long) d) & 0xffff)));
+			 make_save_value (d, 0));
 
   directory_nbytes = SBYTES (directory);
   re_match_object = Qt;
@@ -222,10 +214,15 @@
       errno = 0;
       dp = readdir (d);
 
+      if (dp == NULL && (0
 #ifdef EAGAIN
-      if (dp == NULL && errno == EAGAIN)
-	continue;
+			 || errno == EAGAIN
+#endif
+#ifdef EINTR
+			 || errno == EINTR
 #endif
+			 ))
+	{ QUIT; continue; }
 
       if (dp == NULL)
 	break;
@@ -316,22 +313,11 @@
 	}
     }
 
-  retry_p = 0;
-#ifdef EINTR
-  retry_p |= errno == EINTR;
-#endif
-
   closedir (d);
 
   /* Discard the unwind protect.  */
   specpdl_ptr = specpdl + count;
 
-  if (retry_p)
-    {
-      list = Qnil;
-      goto retry;
-    }
-
   if (NILP (nosort))
     list = Fsort (Fnreverse (list),
 		  attrs ? Qfile_attributes_lessp : Qstring_lessp);
@@ -519,8 +505,7 @@
 	report_file_error ("Opening directory", Fcons (dirname, Qnil));
 
       record_unwind_protect (directory_files_internal_unwind,
-                             Fcons (make_number (((unsigned long) d) >> 16),
-                                    make_number (((unsigned long) d) & 0xffff)));
+                             make_save_value (d, 0));
 
       /* Loop reading blocks */
       /* (att3b compiler bug requires do a null comparison this way) */
@@ -532,8 +517,19 @@
 #ifdef VMS
 	  dp = (*readfunc) (d);
 #else
+	  errno = 0;
 	  dp = readdir (d);
+	  if (dp == NULL && (0
+# ifdef EAGAIN
+			     || errno == EAGAIN
+# endif
+# ifdef EINTR
+			     || errno == EINTR
+# endif
+			     ))
+	    { QUIT; continue; }
 #endif
+
 	  if (!dp) break;
 
 	  len = NAMLEN (dp);