From ad9f5880502c9a3f8e7f8919336888ee056f17ba Mon Sep 17 00:00:00 2001 From: Akira TAGOH Date: Fri, 14 Aug 2015 17:17:34 +0900 Subject: [PATCH] Fix the broken cache more. Take a look at the nano second in the mtime to figure out if the cache needs to be updated if available. and do the mutex lock between scanning and writing a cache to avoid the conflict. Also we don't need to scan directories again after writing caches. so getting rid of the related code as well. https://bugs.freedesktop.org/show_bug.cgi?id=69845 and for reference: https://bugzilla.redhat.com/show_bug.cgi?id=1236034 --- configure.ac | 3 ++ fc-cache/fc-cache.c | 77 ++++++++++++-------------------- fontconfig/fontconfig.h | 2 +- src/fcarch.c | 2 +- src/fccache.c | 98 ++++++++++++++++++++++++++++++++++++++++- src/fcdir.c | 6 +++ src/fcint.h | 8 ++++ 7 files changed, 143 insertions(+), 53 deletions(-) diff --git a/configure.ac b/configure.ac index e888f2c..0ab3cfc 100644 --- a/configure.ac +++ b/configure.ac @@ -166,6 +166,9 @@ dnl posix_fadvise() may be not available in older libc. AC_CHECK_SYMBOL([posix_fadvise], [fcntl.h], [fc_func_posix_fadvise=1], [fc_func_posix_fadvise=0]) AC_DEFINE_UNQUOTED([HAVE_POSIX_FADVISE], [$fc_func_posix_fadvise], [Define to 1 if you have the 'posix_fadvise' function.]) +# +AC_CHECK_MEMBERS([struct stat.st_mtim],,, [#include ]) + # if test "x$ac_cv_func_fstatvfs" = "xyes"; then AC_CHECK_MEMBERS([struct statvfs.f_basetype, struct statvfs.f_fstypename],,, diff --git a/fc-cache/fc-cache.c b/fc-cache/fc-cache.c index 18cd6c5..0336073 100644 --- a/fc-cache/fc-cache.c +++ b/fc-cache/fc-cache.c @@ -122,7 +122,7 @@ usage (char *program, int error) static FcStrSet *processed_dirs; static int -scanDirs (FcStrList *list, FcConfig *config, FcBool force, FcBool really_force, FcBool verbose, FcBool recursive, FcBool error_on_no_fonts, int *changed, FcStrSet *updateDirs) +scanDirs (FcStrList *list, FcConfig *config, FcBool force, FcBool really_force, FcBool verbose, FcBool error_on_no_fonts, int *changed) { int ret = 0; const FcChar8 *dir; @@ -142,15 +142,13 @@ scanDirs (FcStrList *list, FcConfig *config, FcBool force, FcBool really_force, { if (verbose) { - if (!recursive) - printf ("Re-scanning "); if (sysroot) printf ("[%s]", sysroot); printf ("%s: ", dir); fflush (stdout); } - if (recursive && FcStrSetMember (processed_dirs, dir)) + if (FcStrSetMember (processed_dirs, dir)) { if (verbose) printf ("skipping, looped directory detected\n"); @@ -194,13 +192,8 @@ scanDirs (FcStrList *list, FcConfig *config, FcBool force, FcBool really_force, if (!cache) { - if (!recursive) - cache = FcDirCacheRescan (dir, config); - else - { - (*changed)++; - cache = FcDirCacheRead (dir, FcTrue, config); - } + (*changed)++; + cache = FcDirCacheRead (dir, FcTrue, config); if (!cache) { fprintf (stderr, "%s: error scanning\n", dir); @@ -229,37 +222,30 @@ scanDirs (FcStrList *list, FcConfig *config, FcBool force, FcBool really_force, } } - if (recursive) + subdirs = FcStrSetCreate (); + if (!subdirs) { - subdirs = FcStrSetCreate (); - if (!subdirs) - { - fprintf (stderr, "%s: Can't create subdir set\n", dir); - ret++; - FcDirCacheUnload (cache); - continue; - } - for (i = 0; i < FcCacheNumSubdir (cache); i++) - FcStrSetAdd (subdirs, FcCacheSubdir (cache, i)); - if (updateDirs && FcCacheNumSubdir (cache) > 0) - FcStrSetAdd (updateDirs, dir); - + fprintf (stderr, "%s: Can't create subdir set\n", dir); + ret++; FcDirCacheUnload (cache); - - sublist = FcStrListCreate (subdirs); - FcStrSetDestroy (subdirs); - if (!sublist) - { - fprintf (stderr, "%s: Can't create subdir list\n", dir); - ret++; - continue; - } - FcStrSetAdd (processed_dirs, dir); - ret += scanDirs (sublist, config, force, really_force, verbose, recursive, error_on_no_fonts, changed, updateDirs); - FcStrListDone (sublist); + continue; } - else - FcDirCacheUnload (cache); + for (i = 0; i < FcCacheNumSubdir (cache); i++) + FcStrSetAdd (subdirs, FcCacheSubdir (cache, i)); + + FcDirCacheUnload (cache); + + sublist = FcStrListCreate (subdirs); + FcStrSetDestroy (subdirs); + if (!sublist) + { + fprintf (stderr, "%s: Can't create subdir list\n", dir); + ret++; + continue; + } + FcStrSetAdd (processed_dirs, dir); + ret += scanDirs (sublist, config, force, really_force, verbose, error_on_no_fonts, changed); + FcStrListDone (sublist); } if (error_on_no_fonts && !was_processed) ret++; @@ -290,7 +276,7 @@ cleanCacheDirectories (FcConfig *config, FcBool verbose) int main (int argc, char **argv) { - FcStrSet *dirs, *updateDirs; + FcStrSet *dirs; FcStrList *list; FcBool verbose = FcFalse; FcBool force = FcFalse; @@ -393,18 +379,9 @@ main (int argc, char **argv) return 1; } - updateDirs = FcStrSetCreate (); changed = 0; - ret = scanDirs (list, config, force, really_force, verbose, FcTrue, error_on_no_fonts, &changed, updateDirs); - /* Update the directory cache again to avoid the race condition as much as possible */ + ret = scanDirs (list, config, force, really_force, verbose, error_on_no_fonts, &changed); FcStrListDone (list); - list = FcStrListCreate (updateDirs); - if (list) - { - ret += scanDirs (list, config, FcTrue, FcFalse, verbose, FcFalse, error_on_no_fonts, &changed, NULL); - FcStrListDone (list); - } - FcStrSetDestroy (updateDirs); /* * Try to create CACHEDIR.TAG anyway. diff --git a/fontconfig/fontconfig.h b/fontconfig/fontconfig.h index a570b2f..57c1b68 100644 --- a/fontconfig/fontconfig.h +++ b/fontconfig/fontconfig.h @@ -66,7 +66,7 @@ typedef int FcBool; * it means multiple copies of the font information. */ -#define FC_CACHE_VERSION_NUMBER 6 +#define FC_CACHE_VERSION_NUMBER 7 #define _FC_STRINGIFY_(s) #s #define _FC_STRINGIFY(s) _FC_STRINGIFY_(s) #define FC_CACHE_VERSION _FC_STRINGIFY(FC_CACHE_VERSION_NUMBER) diff --git a/src/fcarch.c b/src/fcarch.c index 4a921c0..da33a83 100644 --- a/src/fcarch.c +++ b/src/fcarch.c @@ -48,7 +48,7 @@ FC_ASSERT_STATIC (0x08 + 1*FC_MAX(SIZEOF_VOID_P,ALIGNOF_DOUBLE) == sizeof (FcVal FC_ASSERT_STATIC (0x00 + 2*SIZEOF_VOID_P == sizeof (FcPatternElt)); FC_ASSERT_STATIC (0x08 + 2*SIZEOF_VOID_P == sizeof (FcPattern)); FC_ASSERT_STATIC (0x08 + 2*SIZEOF_VOID_P == sizeof (FcCharSet)); -FC_ASSERT_STATIC (0x08 + 6*SIZEOF_VOID_P == sizeof (FcCache)); +FC_ASSERT_STATIC (0x08 + 7*SIZEOF_VOID_P == sizeof (FcCache)); int diff --git a/src/fccache.c b/src/fccache.c index fc3ed41..20c4418 100644 --- a/src/fccache.c +++ b/src/fccache.c @@ -253,6 +253,7 @@ struct _FcCacheSkip { dev_t cache_dev; ino_t cache_ino; time_t cache_mtime; + long cache_mtime_nano; FcCacheSkip *next[1]; }; @@ -380,12 +381,18 @@ FcCacheInsert (FcCache *cache, struct stat *cache_stat) s->cache_dev = cache_stat->st_dev; s->cache_ino = cache_stat->st_ino; s->cache_mtime = cache_stat->st_mtime; +#ifdef HAVE_STRUCT_STAT_ST_MTIM + s->cache_mtime_nano = cache_stat->st_mtim.tv_nsec; +#else + s->cache_mtime_nano = 0; +#endif } else { s->cache_dev = 0; s->cache_ino = 0; s->cache_mtime = 0; + s->cache_mtime_nano = 0; } /* @@ -473,6 +480,10 @@ FcCacheFindByStat (struct stat *cache_stat) s->cache_ino == cache_stat->st_ino && s->cache_mtime == cache_stat->st_mtime) { +#ifdef HAVE_STRUCT_STAT_ST_MTIM + if (s->cache_mtime != cache_stat->st_mtim.tv_nsec) + continue; +#endif FcRefInc (&s->ref); unlock_cache (); return s->cache; @@ -540,6 +551,7 @@ static FcBool FcCacheTimeValid (FcConfig *config, FcCache *cache, struct stat *dir_stat) { struct stat dir_static; + FcBool fnano = FcTrue; if (!dir_stat) { @@ -558,10 +570,18 @@ FcCacheTimeValid (FcConfig *config, FcCache *cache, struct stat *dir_stat) FcStrFree (d); dir_stat = &dir_static; } +#ifdef HAVE_STRUCT_STAT_ST_MTIM + fnano = (cache->checksum_nano == dir_stat->st_mtim.tv_nsec); + if (FcDebug () & FC_DBG_CACHE) + printf ("FcCacheTimeValid dir \"%s\" cache checksum %d.%ld dir checksum %d.%ld\n", + FcCacheDir (cache), cache->checksum, cache->checksum_nano, (int) dir_stat->st_mtime, dir_stat->st_mtim.tv_nsec); +#else if (FcDebug () & FC_DBG_CACHE) printf ("FcCacheTimeValid dir \"%s\" cache checksum %d dir checksum %d\n", FcCacheDir (cache), cache->checksum, (int) dir_stat->st_mtime); - return cache->checksum == (int) dir_stat->st_mtime; +#endif + + return cache->checksum == (int) dir_stat->st_mtime && fnano; } static FcBool @@ -757,6 +777,10 @@ FcDirCacheValidateHelper (FcConfig *config, int fd, struct stat *fd_stat, struct ret = FcFalse; else if (c.checksum != (int) dir_stat->st_mtime) ret = FcFalse; +#ifdef HAVE_STRUCT_STAT_ST_MTIM + else if (c.checksum_nano != dir_stat->st_mtim.tv_nsec) + ret = FcFalse; +#endif return ret; } @@ -831,6 +855,9 @@ FcDirCacheBuild (FcFontSet *set, const FcChar8 *dir, struct stat *dir_stat, FcSt cache->version = FC_CACHE_VERSION_NUMBER; cache->size = serialize->size; cache->checksum = (int) dir_stat->st_mtime; +#ifdef HAVE_STRUCT_STAT_ST_MTIM + cache->checksum_nano = dir_stat->st_mtim.tv_nsec; +#endif /* * Serialize directory name @@ -1018,6 +1045,11 @@ FcDirCacheWrite (FcCache *cache, FcConfig *config) skip->cache_dev = cache_stat.st_dev; skip->cache_ino = cache_stat.st_ino; skip->cache_mtime = cache_stat.st_mtime; +#ifdef HAVE_STRUCT_STAT_ST_MTIM + skip->cache_mtime_nano = cache_stat.st_mtim.tv_nsec; +#else + skip->cache_mtime_nano = 0; +#endif } unlock_cache (); } @@ -1142,6 +1174,70 @@ FcDirCacheClean (const FcChar8 *cache_dir, FcBool verbose) return ret; } +int +FcDirCacheLock (const FcChar8 *dir, + FcConfig *config) +{ + FcChar8 *cache_hashed = NULL; + FcChar8 cache_base[CACHEBASE_LEN]; + FcStrList *list; + FcChar8 *cache_dir; + const FcChar8 *sysroot = FcConfigGetSysRoot (config); + int fd = -1; + + FcDirCacheBasename (dir, cache_base); + list = FcStrListCreate (config->cacheDirs); + if (!list) + return -1; + + while ((cache_dir = FcStrListNext (list))) + { + if (sysroot) + cache_hashed = FcStrBuildFilename (sysroot, cache_dir, cache_base, NULL); + else + cache_hashed = FcStrBuildFilename (cache_dir, cache_base, NULL); + if (!cache_hashed) + break; + fd = FcOpen ((const char *)cache_hashed, O_RDWR); + /* No caches in that directory. simply retry with another one */ + if (fd != -1) + { + struct flock fl; + + fl.l_type = F_WRLCK; + fl.l_whence = SEEK_SET; + fl.l_start = 0; + fl.l_len = 0; + fl.l_pid = getpid (); + if (fcntl (fd, F_SETLKW, &fl) == -1) + goto bail; + break; + } + } + return fd; +bail: + if (fd != -1) + close (fd); + return -1; +} + +void +FcDirCacheUnlock (int fd) +{ + struct flock fl; + + if (fd != -1) + { + fl.l_type = F_UNLCK; + fl.l_whence = SEEK_SET; + fl.l_start = 0; + fl.l_len = 0; + fl.l_pid = getpid (); + fcntl (fd, F_SETLK, &fl); + close (fd); + } +} + /* * Hokey little macro trick to permit the definitions of C functions * with the same name as CPP macros diff --git a/src/fcdir.c b/src/fcdir.c index 2e7f0dc..f4807dd 100644 --- a/src/fcdir.c +++ b/src/fcdir.c @@ -332,6 +332,7 @@ FcDirCacheScan (const FcChar8 *dir, FcConfig *config) struct stat dir_stat; const FcChar8 *sysroot = FcConfigGetSysRoot (config); FcChar8 *d; + int fd = -1; if (sysroot) d = FcStrBuildFilename (sysroot, dir, NULL); @@ -352,6 +353,7 @@ FcDirCacheScan (const FcChar8 *dir, FcConfig *config) if (!dirs) goto bail1; + fd = FcDirCacheLock (dir, config); /* * Scan the dir */ @@ -371,6 +373,7 @@ FcDirCacheScan (const FcChar8 *dir, FcConfig *config) FcDirCacheWrite (cache, config); bail2: + FcDirCacheUnlock (fd); FcStrSetDestroy (dirs); bail1: FcFontSetDestroy (set); @@ -389,6 +392,7 @@ FcDirCacheRescan (const FcChar8 *dir, FcConfig *config) FcStrSet *dirs; const FcChar8 *sysroot = FcConfigGetSysRoot (config); FcChar8 *d = NULL; + int fd = -1; cache = FcDirCacheLoad (dir, config, NULL); if (!cache) @@ -404,6 +408,7 @@ FcDirCacheRescan (const FcChar8 *dir, FcConfig *config) if (!dirs) goto bail; + fd = FcDirCacheLock (dir, config); /* * Scan the dir */ @@ -422,6 +427,7 @@ FcDirCacheRescan (const FcChar8 *dir, FcConfig *config) FcDirCacheWrite (new, config); bail1: + FcDirCacheUnlock (fd); FcStrSetDestroy (dirs); bail: if (d) diff --git a/src/fcint.h b/src/fcint.h index 15e22fd..68f9817 100644 --- a/src/fcint.h +++ b/src/fcint.h @@ -369,6 +369,7 @@ struct _FcCache { int dirs_count; /* number of subdir strings */ intptr_t set; /* offset to font set */ int checksum; /* checksum of directory state */ + long checksum_nano; /* checksum of directory state */ }; #undef FcCacheDir @@ -590,6 +591,13 @@ FcCacheFini (void); FcPrivate void FcDirCacheReference (FcCache *cache, int nref); +FcPrivate int +FcDirCacheLock (const FcChar8 *dir, + FcConfig *config); + +FcPrivate void +FcDirCacheUnlock (int fd); + /* fccfg.c */ FcPrivate FcBool