Bug 1603801 [patch] Avoid dcache pollution from sdb_measureAccess() r=mt
authorRobert Relyea <rrelyea@redhat.com>
Sat, 18 Apr 2020 17:11:20 -0700 (2020-04-19)
changeset 15570 928721f7016463b6aefef8239ba204bd809416c5
parent 15569 25006e23a7773b1b1569c798210d11bf3f3cc7c4
child 15571 2d66bd9dcad4619bb0e85aa1d9e43eb8ec95dc0e
push id3721
push userrrelyea@redhat.com
push dateMon, 20 Apr 2020 20:32:21 +0000 (2020-04-20)
reviewersmt
bugs1603801
Bug 1603801 [patch] Avoid dcache pollution from sdb_measureAccess() r=mt As implemented, when sdb_measureAccess() runs it creates up to 10,000 negative dcache entries (cached nonexistent filenames). There is no advantage to leaving these particular filenames in the cache; they will never be searched again. Subsequent runs will run a new test with an intentionally different set of filenames. This can have detrimental effects on some systems; a massive negative dcache can lead to memory or performance problems. Since not all platforms have a problem with negative dcache entries, this patch is limitted to those platforms that request it at compilie time (Linux is current the only patch that does.) Differential Revision: https://phabricator.services.mozilla.com/D59652
coreconf/Linux.mk
coreconf/config.gypi
lib/softoken/sdb.c
--- a/coreconf/Linux.mk
+++ b/coreconf/Linux.mk
@@ -16,17 +16,17 @@ ifneq ($(OS_TARGET),Android)
 	USE_PTHREADS = 1
 endif
 
 ifeq ($(USE_PTHREADS),1)
 	IMPL_STRATEGY = _PTH
 endif
 
 DEFAULT_COMPILER = gcc
-DEFINES += -D_DEFAULT_SOURCE -D_BSD_SOURCE -D_POSIX_SOURCE
+DEFINES += -D_DEFAULT_SOURCE -D_BSD_SOURCE -D_POSIX_SOURCE -DSQL_MEASURE_USE_TEMP_DIR
 
 ifeq ($(OS_TARGET),Android)
 ifndef ANDROID_NDK
 	$(error Must set ANDROID_NDK to the path to the android NDK first)
 endif
 ifndef ANDROID_TOOLCHAIN_VERSION
 	$(error Must set ANDROID_TOOLCHAIN_VERSION to the requested version number)
 endif
--- a/coreconf/config.gypi
+++ b/coreconf/config.gypi
@@ -357,16 +357,17 @@
           [ 'OS=="linux" or OS=="android"', {
             'defines': [
               'LINUX2_1',
               'LINUX',
               'linux',
               '_DEFAULT_SOURCE', # for <endian.h> functions, strdup, realpath, and getentropy
               '_BSD_SOURCE', # for the above in glibc <= 2.19
               '_POSIX_SOURCE', # for <signal.h>
+              'SQL_MEASURE_USE_TEMP_DIR', # use tmpdir for the access calls
             ],
           }],
           [ 'OS=="dragonfly" or OS=="freebsd"', {
             'defines': [
               'FREEBSD',
             ],
           }],
           [ 'OS=="netbsd"', {
--- a/lib/softoken/sdb.c
+++ b/lib/softoken/sdb.c
@@ -395,46 +395,73 @@ static PRUint32
 sdb_measureAccess(const char *directory)
 {
     PRUint32 i;
     PRIntervalTime time;
     PRIntervalTime delta;
     PRIntervalTime duration = PR_MillisecondsToInterval(33);
     const char *doesntExistName = "_dOeSnotExist_.db";
     char *temp, *tempStartOfFilename;
-    size_t maxTempLen, maxFileNameLen, directoryLength;
-
+    size_t maxTempLen, maxFileNameLen, directoryLength, tmpdirLength = 0;
+#ifdef SDB_MEASURE_USE_TEMP_DIR
+    /*
+     * on some OS's and Filesystems, creating a bunch of files and deleting
+     * them messes up the systems's caching, but if we create the files in
+     * a temp directory which we later delete, then the cache gets cleared
+     * up. This code uses several OS dependent calls, and it's not clear
+     * that temp directory use won't mess up other filesystems and OS caching,
+     * so if you need this for your OS, you can turn on the
+     * 'SDB_MEASURE_USE_TEMP_DIR' define in coreconf
+     */
+    const char template[] = "dbTemp.XXXXXX";
+    tmpdirLength = sizeof(template);
+#endif
     /* no directory, just return one */
     if (directory == NULL) {
         return 1;
     }
 
     /* our calculation assumes time is a 4 bytes == 32 bit integer */
     PORT_Assert(sizeof(time) == 4);
 
     directoryLength = strlen(directory);
 
-    maxTempLen = directoryLength + strlen(doesntExistName) + 1 /* potential additional separator char */
-                 + 11                                          /* max chars for 32 bit int plus potential sign */
-                 + 1;                                          /* zero terminator */
+    maxTempLen = directoryLength + 1       /* dirname + / */
+                 + tmpdirLength            /* tmpdirname includes / */
+                 + strlen(doesntExistName) /* filename base */
+                 + 11                      /* max chars for 32 bit int plus potential sign */
+                 + 1;                      /* zero terminator */
 
-    temp = PORT_Alloc(maxTempLen);
+    temp = PORT_ZAlloc(maxTempLen);
     if (!temp) {
         return 1;
     }
 
     /* We'll copy directory into temp just once, then ensure it ends
-     * with the directory separator, then remember the position after
-     * the separator, and calculate the number of remaining bytes. */
+     * with the directory separator. */
 
     strcpy(temp, directory);
     if (directory[directoryLength - 1] != PR_GetDirectorySeparator()) {
         temp[directoryLength++] = PR_GetDirectorySeparator();
     }
-    tempStartOfFilename = temp + directoryLength;
+
+#ifdef SDB_MEASURE_USE_TEMP_DIR
+    /* add the template for a temporary subdir, and create it */
+    strcat(temp, template);
+    if (!mkdtemp(temp)) {
+        PORT_Free(temp);
+        return 1;
+    }
+    /* and terminate that tmp subdir with a / */
+    strcat(temp, "/");
+#endif
+
+    /* Remember the position after the last separator, and calculate the
+     * number of remaining bytes. */
+    tempStartOfFilename = temp + directoryLength + tmpdirLength;
     maxFileNameLen = maxTempLen - directoryLength;
 
     /* measure number of Access operations that can be done in 33 milliseconds
      * (1/30'th of a second), or 10000 operations, which ever comes first.
      */
     time = PR_IntervalNow();
     for (i = 0; i < 10000u; i++) {
         PRIntervalTime next;
@@ -448,16 +475,22 @@ sdb_measureAccess(const char *directory)
                     ".%lu%s", (PRUint32)(time + i), doesntExistName);
         PR_Access(temp, PR_ACCESS_EXISTS);
         next = PR_IntervalNow();
         delta = next - time;
         if (delta >= duration)
             break;
     }
 
+#ifdef SDB_MEASURE_USE_TEMP_DIR
+    /* turn temp back into our tmpdir path by removing doesntExistName, and
+     * remove the tmp dir */
+    *tempStartOfFilename = '\0';
+    (void)rmdir(temp);
+#endif
     PORT_Free(temp);
 
     /* always return 1 or greater */
     return i ? i : 1u;
 }
 
 /*
  * some file sytems are very slow to run sqlite3 on, particularly if the