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
This commit is contained in:
Akira TAGOH 2015-08-14 17:17:34 +09:00
parent 46ec6a52d4
commit ad9f588050
7 changed files with 143 additions and 53 deletions

View File

@ -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 <sys/stat.h>])
#
if test "x$ac_cv_func_fstatvfs" = "xyes"; then
AC_CHECK_MEMBERS([struct statvfs.f_basetype, struct statvfs.f_fstypename],,,

View File

@ -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");
@ -193,14 +191,9 @@ 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);
}
if (!cache)
{
fprintf (stderr, "%s: error scanning\n", dir);
@ -229,8 +222,6 @@ scanDirs (FcStrList *list, FcConfig *config, FcBool force, FcBool really_force,
}
}
if (recursive)
{
subdirs = FcStrSetCreate ();
if (!subdirs)
{
@ -241,8 +232,6 @@ scanDirs (FcStrList *list, FcConfig *config, FcBool force, FcBool really_force,
}
for (i = 0; i < FcCacheNumSubdir (cache); i++)
FcStrSetAdd (subdirs, FcCacheSubdir (cache, i));
if (updateDirs && FcCacheNumSubdir (cache) > 0)
FcStrSetAdd (updateDirs, dir);
FcDirCacheUnload (cache);
@ -255,12 +244,9 @@ scanDirs (FcStrList *list, FcConfig *config, FcBool force, FcBool really_force,
continue;
}
FcStrSetAdd (processed_dirs, dir);
ret += scanDirs (sublist, config, force, really_force, verbose, recursive, error_on_no_fonts, changed, updateDirs);
ret += scanDirs (sublist, config, force, really_force, verbose, error_on_no_fonts, changed);
FcStrListDone (sublist);
}
else
FcDirCacheUnload (cache);
}
if (error_on_no_fonts && !was_processed)
ret++;
return 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.

View File

@ -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)

View File

@ -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

View File

@ -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

View File

@ -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)

View File

@ -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