[Fontconfig] fontconfig: Branch 'master'

Akira TAGOH tagoh at kemper.freedesktop.org
Fri Aug 14 01:20:29 PDT 2015


 configure.ac            |    3 +
 fc-cache/fc-cache.c     |   75 ++++++++++++------------------------
 fontconfig/fontconfig.h |    2 
 src/fcarch.c            |    2 
 src/fccache.c           |   98 +++++++++++++++++++++++++++++++++++++++++++++++-
 src/fcdir.c             |    6 ++
 src/fcint.h             |    8 +++
 7 files changed, 142 insertions(+), 52 deletions(-)

New commits:
commit ad9f5880502c9a3f8e7f8919336888ee056f17ba
Author: Akira TAGOH <akira at tagoh.org>
Date:   Fri Aug 14 17:17:34 2015 +0900

    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

diff --git a/configure.ac b/configure.ac
index e888f2c..0ab3cfc 100644
--- a/configure.ac
+++ b/configure.ac
@@ -167,6 +167,9 @@ AC_CHECK_SYMBOL([posix_fadvise], [fcntl.h], [fc_func_posix_fadvise=1], [fc_func_
 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],,,
 		[#include <sys/statvfs.h>])
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);
+	    continue;
+	}
+	for (i = 0; i < FcCacheNumSubdir (cache); i++)
+	    FcStrSetAdd (subdirs, FcCacheSubdir (cache, i));
 	
-	    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);
+	FcDirCacheUnload (cache);
+
+	sublist = FcStrListCreate (subdirs);
+	FcStrSetDestroy (subdirs);
+	if (!sublist)
+	{
+	    fprintf (stderr, "%s: Can't create subdir list\n", dir);
+	    ret++;
+	    continue;
 	}
-	else
-	    FcDirCacheUnload (cache);
+	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


More information about the Fontconfig mailing list