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
|