Merge pull request #17636 from lydia-duncan/fixChpldocTmpDir

Fix issues with chpldoc temporary directory
[reviewed by @ronawho]

We started encountering problems in nightly testing where we'd accidentally put
chpldoc temporary files in a folder that already had old chpldoc error files, causing
us to encounter sporadic failures that matched the old error files.  This was because
we were failing to clean up old temporary directories after a failure.  This change
accomplishes two things:
1. It ensures that we clean up the temporary directory for chpldoc, even when we
encounter an error
2. It stops using a homegrown strategy to create the temporary directory, instead
relying on mkdtemp.  We do still perform some computation to determine the start
of the temporary directory's name (involving getting the username and attaching
the program name and "deleteme" to it) but we no longer use the PID to differentiate,
instead relying on mkdtemp's randomization for us.

Resolves Cray/chapel-private#1932
Resolves Cray/chapel-private#1678

With this change, I was no longer seeing leftover chpldoc directories in my temp folder,
and did not see additional chpl directories (because this does alter how we compute the
temporary directory name for chpl compilations, since they share a code path)

<details>
The variable storing the temporary directory for chpldoc was local, which meant
that we'd only remove its contents after a successful run of the compiler.  This
change replaces that variable with a global variable stored in the same location
as the temporary directory used for normal operations, and adjusts the clean up
call at the exit of the compiler's operations to clean it up as well (which runs
regardless of if the compiler ran successfully or encountered an error).

At Elliot's suggestion, use mkdtemp to make a temporary directory, for both
chpldoc and chpl programs.  Because mkdtemp requires its strings end in six Xs,
alter the name we use from "tmpdirloc/chpl[doc]-username-pid.deleteme" to
"tmpdirloc/chpl[doc]-username.deleteme-XXXXXX" and remove all references to the
PID in this setup.  The username stuff seems reasonable to keep and the Xs will
be replaced with a unique enough set of characters for our purposes.
</details>

Testing:
Passed a full paratest with futures and a spot check on my Mac with hello.chpl and
a chpldoc test that had triggered the leftover directory before

compiler/util/tmpdirname.h → compiler/include/tmpdirname.h

@@ -22,5 +22,6 @@
 #define _TMPDIRNAME_H_
 
 extern const char* tmpdirname;
+extern const char* doctmpdirname;
 
 #endif

compiler/passes/docs.cpp

@@ -40,6 +40,7 @@
 #include "stmt.h"
 #include "symbol.h"
 #include "stringutil.h"
+#include "tmpdirname.h"
 
 static int compareNames(const void* v1, const void* v2) {
   Symbol* s1 = *(Symbol* const *)v1;
@@ -68,13 +69,12 @@ void docs(void) {
 
     // Root of the sphinx project and generated rst files. If
     // --docs-save-sphinx is not specified, it will be a temp dir.
-    std::string docsTempDir = "";
     std::string docsSphinxDir;
     if (strlen(fDocsSphinxDir) > 0) {
       docsSphinxDir = fDocsSphinxDir;
     } else {
-      docsTempDir = makeTempDir("chpldoc-");
-      docsSphinxDir = docsTempDir;
+      doctmpdirname = makeTempDir("chpldoc-");
+      docsSphinxDir = doctmpdirname;
     }
 
     // Make the intermediate dir and output dir.
@@ -113,10 +113,6 @@ void docs(void) {
     if (!fDocsTextOnly && fDocsHTML) {
       generateSphinxOutput(docsSphinxDir, docsOutputDir);
     }
-
-    if (docsTempDir.length() > 0) {
-      deleteDir(docsTempDir.c_str());
-    }
   }
 }
 

compiler/util/files.cpp

@@ -72,8 +72,6 @@ std::vector<const char*>   libFiles;
 // directory for intermediates; tmpdir or saveCDir
 static const char* intDirName        = NULL;
 
-static const int   MAX_CHARS_PER_PID = 32;
-
 static void addPath(const char* pathVar, std::vector<const char*>* pathvec) {
   char* dirString = strdup(pathVar);
 
@@ -161,15 +159,7 @@ static const char* getTempDir() {
 
 const char* makeTempDir(const char* dirPrefix) {
   const char* tmpdirprefix = astr(getTempDir(), "/", dirPrefix);
-  const char* tmpdirsuffix = ".deleteme";
-
-  pid_t mypid = getpid();
-#ifdef DEBUGTMPDIR
-  mypid = 0;
-#endif
-
-  char mypidstr[MAX_CHARS_PER_PID];
-  snprintf(mypidstr, MAX_CHARS_PER_PID, "-%d", (int)mypid);
+  const char* tmpdirsuffix = ".deleteme-XXXXXX";
 
   struct passwd* passwdinfo = getpwuid(geteuid());
   const char* userid;
@@ -181,12 +171,12 @@ const char* makeTempDir(const char* dirPrefix) {
   char* myuserid = strdup(userid);
   removeSpacesBackslashesFromString(myuserid);
 
-  const char* tmpDir = astr(tmpdirprefix, myuserid, mypidstr, tmpdirsuffix);
-  ensureDirExists(tmpDir, "making temporary directory");
+  const char* tmpDir = astr(tmpdirprefix, myuserid, tmpdirsuffix);
+  const char* dirRes = mkdtemp((char*) tmpDir);
 
   free(myuserid); myuserid = NULL;
 
-  return tmpDir;
+  return dirRes;
 }
 
 static void ensureTmpDirExists() {
@@ -254,6 +244,15 @@ void deleteTmpDir() {
     deleteDir(tmpdirname);
     tmpdirname = NULL;
   }
+  if (doctmpdirname != NULL) {
+    if (strlen(doctmpdirname) < 1 ||
+        strchr(doctmpdirname, '*') != NULL ||
+        strcmp(doctmpdirname, "//") == 0) {
+      INT_FATAL("doc tmp directory name looks fishy");
+    }
+    deleteDir(doctmpdirname);
+    doctmpdirname = NULL;
+  }
 #endif
 
   inDeleteTmpDir = 0;

compiler/util/tmpdirname.cpp

@@ -26,6 +26,7 @@
 // IF tmpdirname's name CHANGES, IT NEEDS TO CHANGE IN createGDBFile AS WELL
 //
 const char* tmpdirname = NULL;
+const char* doctmpdirname = NULL;
 //
 //          ^^^^^^^^^^
 //