Print

---

On Mon, 23 Apr 2007, Jan Just Keijser wrote:

> hmmm
> readdir is crashing in a multithreaded application... shouldn't we be 
> using readdir_r and readdir64_r when using threads? I've looked at the 
> original gridmapdir patch and indeed, it lists
> 
> +void
> +gridmapdir_newlease(char *     encodedglobusidp,
> +                    char *     usernameprefix)
> +{
> +     int            ret;
> +     char           *userfilename, *encodedfilename, *gridmapdir;
> +     struct dirent  *gridmapdirentry;
> +     DIR            *gridmapdirstream;
> +     struct stat    statbuf;
> +     
> +     gridmapdir = getenv("GRIDMAPDIR");
> +     if (gridmapdir == NULL) return;
> +
> +     encodedfilename = malloc(strlen(gridmapdir) + (size_t) 2 + 
> +                              strlen(encodedglobusidp));
> +     sprintf(encodedfilename, "%s/%s", gridmapdir, encodedglobusidp);
> +
> +     gridmapdirstream = opendir(gridmapdir);
> +
> +     while ((gridmapdirentry = readdir(gridmapdirstream)) != NULL)
> 
> 
> which indeed, is not threadsafe...
> so this is a "sleeping" bug that can hit anybody as soon as
> - the system is under heavy load
> - the gridmapdir gets too large
> 
> how can we either
> - limit the number of threads of the network-server
> - patch and recompile the vdt_globus_essentials package, with gridmapdir 
> patch

A patched rpm has been tested and is available here for the time being:

    http://litmaath.home.cern.ch/litmaath/gmd-fix/

The diffs:

-----------------------------------------------------------------------------
--- gridmap.c.orig	2004-05-25 23:04:18.000000000 +0200
+++ gridmap.c	2007-04-23 23:06:40.000000000 +0200
@@ -121,7 +121,7 @@
      int            ret;
      char           *firstlinkpath, *otherlinkdup, *otherlinkpath,
                     *gridmapdir;
-     struct dirent  *gridmapdirentry;
+     struct dirent  *gridmapdirentry = 0, gmde;
      DIR            *gridmapdirstream;
      struct stat    statbuf;
      ino_t          firstinode;
@@ -142,7 +142,8 @@
 
      if (gridmapdirstream != NULL)
      {
-         while ((gridmapdirentry = readdir(gridmapdirstream)) != NULL)
+	 while (readdir_r(gridmapdirstream, &gmde, &gridmapdirentry) == 0 &&
+		gridmapdirentry != NULL)
          {       
                  if (strcmp(gridmapdirentry->d_name, firstlink) == 0) continue;
            
@@ -238,7 +239,7 @@
 {
      int            ret;
      char           *userfilename, *encodedfilename, *gridmapdir;
-     struct dirent  *gridmapdirentry;
+     struct dirent  *gridmapdirentry = 0, gmde;
      DIR            *gridmapdirstream;
      struct stat    statbuf;
      
@@ -251,7 +252,8 @@
 
      gridmapdirstream = opendir(gridmapdir);
 
-     while ((gridmapdirentry = readdir(gridmapdirstream)) != NULL)
+     while (readdir_r(gridmapdirstream, &gmde, &gridmapdirentry) == 0 &&
+            gridmapdirentry != NULL)
      {
        /* we dont want any files that dont look like acceptable usernames */
        if ((*(gridmapdirentry->d_name) == '%') || 
-----------------------------------------------------------------------------

I will submit a bug and a patch, which should be certified fairly soon.
Thanks,
	Maarten