Fontconfig - Jan 2006 - Crash when non-existing directories are referenced in ~/.fonts.cache

How to reproduce:

Delete global caches and ~/.fonts.cache-2:

    mfabian@magellan:~$ sudo rm -f /var/cache/fontconfig/*
    mfabian@magellan:~$ rm -f .fonts.cache*
    mfabian@magellan:~$

Create ~/.fonts.cache-2 again:

    mfabian@magellan:~$ time fc-match sans 
    hgjgbbmp.ttc: "HGPGothicB" "Regular"

    real	3m55.605s
    user	0m59.556s
    sys	0m2.912s
    mfabian@magellan:~$

Move one directory which has been added to ~/.fonts.cache-2 out
of the way:

    mfabian@magellan:~$ sudo mv /usr/share/fonts /tmp
    mfabian@magellan:~$

~/.fonts.cache-2 now contains references to a
non-existing directory:

    mfabian@magellan:~$ fc-cat .fonts.cache-2 | grep usr/share/fonts
    fc-cat: printing global cache contents for dir /usr/share/fonts/wine
    fc-cat: printing global cache contents for dir /usr/share/fonts/bdf
    fc-cat: printing global cache contents for dir /usr/share/fonts
    mfabian@magellan:~$ 

Try to use a fontconfig application, e.g. "fc-match":

    mfabian@magellan:~$ time fc-match sans
    ????????????? (core dumped)

    real	0m16.670s
    user	0m0.024s
    sys	0m0.152s
    mfabian@magellan:~$ 

Crashes.

-- 
Mike FABIAN   <mfabian@suse.de>   http://www.suse.de/~mfabian
?????????????

There''s a bunch of related crashes that I''m trying to fix and
will
hopefully commit soon.  Please wait till after I commit them and we''ll
see which crashes remain.

pat

Patrick Lam <plam@MIT.EDU> ????????:
> There''s a bunch of related crashes that I''m trying to fix
and will
> hopefully commit soon.  Please wait till after I commit them and
we''ll
> see which crashes remain.
As I have already a patch ready which fixes this, I''ll attach
it here.

-------------- next part --------------
A non-text attachment was scrubbed...
Name:
do-not-crash-if-non-existing-directories-are-referenced-from-local-cache-file.patch
Type: text/x-patch
Size: 627 bytes
Desc: not available
Url :
http://lists.freedesktop.org/archives/fontconfig/attachments/20060126/ed410bce/do-not-crash-if-non-existing-directories-are-referenced-from-local-cache-file.bin
-------------- next part --------------

-- 
Mike FABIAN   <mfabian@suse.de>   http://www.suse.de/~mfabian
?????????????

Mike FABIAN wrote:
> Patrick Lam <plam@MIT.EDU> ????????:
> 
> 
>>There''s a bunch of related crashes that I''m trying to
fix and will
>>hopefully commit soon.  Please wait till after I commit them and
we''ll
>>see which crashes remain.
> 
> 
> As I have already a patch ready which fixes this, I''ll attach
> it here.
I''ve committed this patch.

I''ve also committed a patch which makes the handling of cache files
less
broken in general (in particular, this should fix the fc-cat weirdness;
the cache writing code was trampling the directory name being to the
file.)  So things should be somewhat better.

There''s unfortunately still problems with cache files; I still get a
crash when I run fc-cache twice.  I''ll try to fix that this afternoon.

pat

The following code in FcConfigNormalizeFontDir ()

     /* Ok, we didn''t find it in fontDirs; let''s add
subdirs.... */
     for (n = 0; n < config->fontDirs->num; n++)
 	FcConfigAddFontDirSubdirs (config, config->fontDirs->strs[n]);

adds the same subdirectories many times because 
config->fontDirs->num is getting bigger during the for loop.

This causes many unnecessary opendir() and readdir() calls.

I think one should end the for loop when the initial value of
config->fontDirs->num is reached. All values in the list after the
initial value of num are newly added subdirectories, there is no
reason to scan them for subdirectories again.

Patch attached.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: nmax.patch
Type: text/x-patch
Size: 775 bytes
Desc: not available
Url :
http://lists.freedesktop.org/archives/fontconfig/attachments/20060126/642407c8/nmax.bin
-------------- next part --------------

-- 
Mike FABIAN   <mfabian@suse.de>   http://www.suse.de/~mfabian
?????????????

Quoting Mike FABIAN <mfabian@suse.de>:
> I think one should end the for loop when the initial value of
> config->fontDirs->num is reached. All values in the list after the
> initial value of num are newly added subdirectories, there is no
> reason to scan them for subdirectories again.
Yeah, I''ve caught that and fixed the fc-cache segfault that occurs when
you e.g.
give ''.'' or a trailing slash.  I''ll commit the
changes I have once I
can hook up
my laptop.

pat

Mike FABIAN wrote:
> The following code in FcConfigNormalizeFontDir ()
> 
>      /* Ok, we didn''t find it in fontDirs; let''s add
subdirs.... */
>      for (n = 0; n < config->fontDirs->num; n++)
>  	FcConfigAddFontDirSubdirs (config, config->fontDirs->strs[n]);
> 
> adds the same subdirectories many times because 
> config->fontDirs->num is getting bigger during the for loop.
> 
> This causes many unnecessary opendir() and readdir() calls.
> 
> I think one should end the for loop when the initial value of
> config->fontDirs->num is reached. All values in the list after the
> initial value of num are newly added subdirectories, there is no
> reason to scan them for subdirectories again.
I''ve committed my version of this patch and a fix for fc-cache.

In my experience, fc-cache . and fc-cache with a trailing slash work
correctly now.  Let me know if there are any remaining issues.  I''ll
also try to test it more myself.

pat

tor, 26,.01.2006 kl. 19.31 -0500, skrev Patrick Lam:
> Mike FABIAN wrote:
> > The following code in FcConfigNormalizeFontDir ()
> > 
> >      /* Ok, we didn''t find it in fontDirs; let''s add
subdirs.... */
> >      for (n = 0; n < config->fontDirs->num; n++)
> >  	FcConfigAddFontDirSubdirs (config, config->fontDirs->strs[n]);
> > 
> > adds the same subdirectories many times because 
> > config->fontDirs->num is getting bigger during the for loop.
> > 
> > This causes many unnecessary opendir() and readdir() calls.
> > 
> > I think one should end the for loop when the initial value of
> > config->fontDirs->num is reached. All values in the list after
the
> > initial value of num are newly added subdirectories, there is no
> > reason to scan them for subdirectories again.
> 
> I''ve committed my version of this patch and a fix for fc-cache.
> 
> In my experience, fc-cache . and fc-cache with a trailing slash work
> correctly now.  Let me know if there are any remaining issues. 
I''ll
> also try to test it more myself.
> 
Something broke badly in this commit - fc-match or any app using fc now
loops with "Fontconfig error: Cannot load default config file" untill
they segfault. Reverting to the previous commit things work fine (and
fc-cache no longer creates broken caches - yay!).

-- 
Ronny V. Vindenes <ronnyvv@broadpark.no>

Ronny V. Vindenes wrote:
> Something broke badly in this commit - fc-match or any app using fc now
> loops with "Fontconfig error: Cannot load default config file"
untill
> they segfault. Reverting to the previous commit things work fine (and
> fc-cache no longer creates broken caches - yay!).
Oops, just because fc-cache works doesn''t mean that everything else
does.

I''ve committed a fix for fontconfig-using applications.

make check still fails, but I''ll take a look at that later.  I think
the
problem is that the global cache doesn''t record all subdirectories
properly.

pat

On Fri, 2006-01-27 at 00:53 -0500, Patrick Lam wrote:
> Ronny V. Vindenes wrote:
> > Something broke badly in this commit - fc-match or any app using fc
now
> > loops with "Fontconfig error: Cannot load default config
file" untill
> > they segfault. Reverting to the previous commit things work fine (and
> > fc-cache no longer creates broken caches - yay!).
> 
> Oops, just because fc-cache works doesn''t mean that everything
else does.
> 
> I''ve committed a fix for fontconfig-using applications.
> 
> make check still fails, but I''ll take a look at that later.  I
think the
> problem is that the global cache doesn''t record all subdirectories
properly.
> 
> pat
With current cvs, fc-cat is still busted:
> rm ~/.fonts.cache-2
> fc-list :
> ls -l ~/.fonts.cache-2
-rw------- 1 mclasen mclasen 141434 Jan 27 10:13
/home/boston/mclasen/.fonts.cache-2
> fc-cat ~/.fonts.cache-2 | wc -l
144
> fc-list :
> ls -l ~/.fonts.cache-2
-rw------- 1 mclasen mclasen 32768 Jan 27 10:03
/home/boston/mclasen/.fonts.cache-2
> fc-cat ~/.fonts.cache-2 
[fc-cat goes in an infinite loop]


It seems to me that all the recent breakage is due to the global cache. 
It just complicates the caching logic a lot. Would be it be too hard to just
demand
fc-cache being run after installing new fonts ? We do that for everything else
thats
cached, like icon themes, mime data, etc...

Matthias

(Once again without the broken ~/.fonts.cache-2 file attached
because the message size was too big)

If this broken ~/.fonts.cache file exists (and no cache files are in
/var/cache/fontconfig) fontconfig (e.g. fc-match) loops endlessly,
eating up all system resources.

mfabian@magellan:~$ fc-cat .fonts.cache-2 | grep printing
fc-cat: printing global cache contents for dir /usr/share/fonts/wine
fc-cat: printing global cache contents for dir /usr/share/fonts
fc-cat: printing global cache contents for dir /usr/share/fonts/bdf
fc-cat: printing global cache contents for dir
tmp.xemacs-patches.200505:xemacs-patches.200505.SCORE?
mfabian@magellan:~$ 

Don''t ask me how that strange last "directory" which
doesn''t really
exist of course got into that cache file. That must be due to yet
another weird bug in fontconfig. I have a *file* (not a directory!)
with a similar name in the "News" folder in my home directory:

mfabian@magellan:~$ ls News/*xemacs-patches.200505*
News/nndoc+.tmp.xemacs-patches.200505:xemacs-patches.200505.SCORE
mfabian@magellan:~$ 

But fontconfig has no business in ~/News, it is not listed as a
font directory in any config file.

Anyway, when reading this broken cache file, fontconfig loops
endlessly because the return value of FcCacheReadString () is not
checked in fccache.c around line 242:

	FcCacheReadString (cache->fd, name_buf, sizeof (name_buf));
	if (!strlen(name_buf))
	    break;

The return value of FcCacheReadString () is zero because the end of
the cache file has been reached but "name_buf" still contains the
previous contents, therefore it never breaks (by the way, it might be
a good idea to check the return value of the read () a few lines later
as well).

I''ve tried to fix this (and also tried to check the return value
at the other places where FcCacheReadString () was called, just
to make sure).

Patch attached.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: endless-loop.patch
Type: text/x-patch
Size: 2633 bytes
Desc: not available
Url :
http://lists.freedesktop.org/archives/fontconfig/attachments/20060127/86889a8c/endless-loop.bin
-------------- next part --------------

-- 
Mike FABIAN   <mfabian@suse.de>   http://www.suse.de/~mfabian
?????????????

I have internet access again and can commit some patches...

Matthias Clasen wrote:
> [fc-cat goes in an infinite loop]
It looks like Mike Fabian got that one.  I''ll take a look at it now.
> It seems to me that all the recent breakage is due to the global cache. 
> It just complicates the caching logic a lot. Would be it be too hard to
just demand
> fc-cache being run after installing new fonts ? We do that for everything
else thats
> cached, like icon themes, mime data, etc...
The recent breakage is, in my opinion, due to putting directory caches
into /var/cache/fontconfig and the resulting changes in global and
directory cache files needed to support this.  I''m not sure that this
was, in fact, an improvement, and complicated a lot of the code; it
helps satisfy some FHS constraint, but seems to be hard to get right and
is still conceptually a bit problematic.  It was much simpler to put
font caches in their own directories.

I don''t think it''s appropriate to demand that fc-cache be run
after
installing new fonts, especially since fc-cache runs as root and users
can install their own fonts.  That would make fontconfig worse than it
is now.

pat

On Sun, 2006-01-29 at 23:36 -0500, Patrick Lam wrote:
> I have internet access again and can commit some patches...
> 
> Matthias Clasen wrote:
> > [fc-cat goes in an infinite loop]
> 
> It looks like Mike Fabian got that one.  I''ll take a look at it
now.
> 
> > It seems to me that all the recent breakage is due to the global
cache.
> > It just complicates the caching logic a lot. Would be it be too hard
to just demand
> > fc-cache being run after installing new fonts ? We do that for
everything else thats
> > cached, like icon themes, mime data, etc...
> 
> The recent breakage is, in my opinion, due to putting directory caches
> into /var/cache/fontconfig and the resulting changes in global and
> directory cache files needed to support this.  I''m not sure that
this
> was, in fact, an improvement, and complicated a lot of the code; it
> helps satisfy some FHS constraint, but seems to be hard to get right and
> is still conceptually a bit problematic.  It was much simpler to put
> font caches in their own directories.
> 
> I don''t think it''s appropriate to demand that fc-cache be
run after
> installing new fonts, especially since fc-cache runs as root and users
> can install their own fonts.  That would make fontconfig worse than it
> is now.
Well, the current state of continuous recreation of $HOME/.fonts.cache
is certainly worse than having to run fc-cache after font installation. 
fc-cache does not have to be run as root, I can use it just fine as an
ordinary user to create a font cache in $HOME/.fonts.

But regardless of this discussion, I''m still seeing some problems with
current CVS. Repeatedly running fc-list : makes $HOME/.fonts.cache-2
change its size (for me it oscillates between 141434 and 141538), so 
it is obviously being rewritten every time, even though the set of 
installed fonts does not change at all.

fc-cat does not list any fonts when run on $HOME/.fonts.cache-2.
When run on one of the directory caches in /var/cache/fontconfig, it
produces garbage in the charset fields.

Matthias

Matthias Clasen <mclasen@redhat.com> ????????:
> But regardless of this discussion, I''m still seeing some problems
with
> current CVS. Repeatedly running fc-list : makes $HOME/.fonts.cache-2
> change its size (for me it oscillates between 141434 and 141538), so 
> it is obviously being rewritten every time, even though the set of 
> installed fonts does not change at all.
I had this problem previously, but it didn''t happen always and
somehow it seems to be not so easy to reproduce.

Luckily I can reproduce this oscillation problem right now (with todays
CVS). I.e. I have a chance to investigate this now. 

-- 
Mike FABIAN   <mfabian@suse.de>   http://www.suse.de/~mfabian
?????????????

Mike FABIAN wrote:
> The return value of FcCacheReadString () is zero because the end of
> the cache file has been reached but "name_buf" still contains the
> previous contents, therefore it never breaks (by the way, it might be
> a good idea to check the return value of the read () a few lines later
> as well).
Looks good to me.  I''ve committed this patch.  Maybe it''ll
help with the
weird .fonts.cache-2 cyclic behaviour too.

pat

Matthias Clasen wrote:
> Well, the current state of continuous recreation of $HOME/.fonts.cache
> is certainly worse than having to run fc-cache after font installation. 
> fc-cache does not have to be run as root, I can use it just fine as an
> ordinary user to create a font cache in $HOME/.fonts.
Yes.  However, that cache is just a cache for the .fonts directory, not 
for the system fonts.
> But regardless of this discussion, I''m still seeing some problems
with
> current CVS. Repeatedly running fc-list : makes $HOME/.fonts.cache-2
> change its size (for me it oscillates between 141434 and 141538), so 
> it is obviously being rewritten every time, even though the set of 
> installed fonts does not change at all.
> 
> fc-cat does not list any fonts when run on $HOME/.fonts.cache-2.
> When run on one of the directory caches in /var/cache/fontconfig, it
> produces garbage in the charset fields.
I''ve fixed the fc-cat bug for the global cache.  I haven''t
observed any
garbage in fc-cat on charset fields.  Currently fontconfig isn''t 
properly reading the global cache, so I''m taking a look at this.

pat

Patrick Lam <plam@MIT.EDU> ????????:
> Mike FABIAN wrote:
>> The return value of FcCacheReadString () is zero because the end of
>> the cache file has been reached but "name_buf" still contains
the
>> previous contents, therefore it never breaks (by the way, it might be
>> a good idea to check the return value of the read () a few lines later
>> as well).
>
> Looks good to me.  I''ve committed this patch.  Maybe
it''ll help with the
> weird .fonts.cache-2 cyclic behaviour too.
Unfortunaly not. I still could reproduce the cyclic behaviour today
even with my patch applied.

-- 
Mike FABIAN   <mfabian@suse.de>   http://www.suse.de/~mfabian
?????????????

Le lundi 30 janvier 2006 ? 11:03 -0500, Patrick Lam a ?crit
:
> Matthias Clasen wrote:
> > Well, the current state of continuous recreation of $HOME/.fonts.cache
> > is certainly worse than having to run fc-cache after font
installation.
> > fc-cache does not have to be run as root, I can use it just fine as an
> > ordinary user to create a font cache in $HOME/.fonts.
> 
> Yes.  However, that cache is just a cache for the .fonts directory, not 
> for the system fonts.
Unless some system fonts are not cached properly (if fc-cache wasn''t
run
as root), in that case, they will be cached in ~/.fonts.cache-2.

-- 
Frederic Crozat <fcrozat@mandriva.com>
Mandriva

Mike FABIAN wrote:
> Patrick Lam <plam@MIT.EDU> ????????:
> 
> 
>>Mike FABIAN wrote:
>>
>>>The return value of FcCacheReadString () is zero because the end of
>>>the cache file has been reached but "name_buf" still
contains the
>>>previous contents, therefore it never breaks (by the way, it might
be
>>>a good idea to check the return value of the read () a few lines
later
>>>as well).
>>
>>Looks good to me.  I''ve committed this patch.  Maybe
it''ll help with the
>>weird .fonts.cache-2 cyclic behaviour too.
> 
> Unfortunaly not. I still could reproduce the cyclic behaviour today
> even with my patch applied.
There was also some really bad behaviour with the global cache which I
just fixed.  That should not have addressed the cyclic behaviour, but
now the global caches might actually work again.

pat

Patrick Lam <plam@MIT.EDU> ????????:
> Mike FABIAN wrote:
>> Patrick Lam <plam@MIT.EDU> ????????:
>> 
>> 
>>>Mike FABIAN wrote:
>>>
>>>>The return value of FcCacheReadString () is zero because the end
of
>>>>the cache file has been reached but "name_buf" still
contains the
>>>>previous contents, therefore it never breaks (by the way, it
might be
>>>>a good idea to check the return value of the read () a few lines
later
>>>>as well).
>>>
>>>Looks good to me.  I''ve committed this patch.  Maybe
it''ll help with the
>>>weird .fonts.cache-2 cyclic behaviour too.
>> 
>> Unfortunaly not. I still could reproduce the cyclic behaviour today
>> even with my patch applied.
>
> There was also some really bad behaviour with the global cache which I
> just fixed.  That should not have addressed the cyclic behaviour, but
> now the global caches might actually work again.
I cannot reproduce the cyclic problem now. Maybe fixed by accident.

-- 
Mike FABIAN   <mfabian@suse.de>   http://www.suse.de/~mfabian
?????????????