[Fontconfig] The reference counter handling in FcConfigGetCurrent() and FcConfigReference().

[mpsuzuki at hiroshima-u.ac.jp]

Dear fontconfig maintainers,

FcConfig object includes its reference counter, it prevents
that FcConfigDestroy() frees an object in use. I think it's
good design.

In the current implementation, I'm questionable about 2 points:

P1) FcConfigGetCurrent() does not increment the reference
    counter when it returns existing FcConfig object.

P2) FcConfigReference() increments the reference counter
    even when it returns new object created by FcConfigGetCurrent().

By P1, some libraries linking other libraries which uses
fontconfig too cannot free FcConfig object safely.

For example, poppler links fontconfig by itself (to import
font resources out of the document), and sometimes it links
cairo (to render PDF to scalable graphics) which links
fontconfig too.

The poppler invokes FcConfigGetCurrent() when it reads
given PDF document, and sometimes cairo invoked by poppler
invokes FcConfigGetCurrent() again, to write an output file.
Unfortunately, the dependency of poppler onto cairo is
not essential, so, poppler does not pass FcConfig object
to cairo. Also no cairo API takes FcConfig object from
the caller.

How we can free the object obtained by FcConfigGetCurrent()?
In current implementation, we cannot know how many clients
invoked FcConfigGetCurrent() from FcConfig->ref, so invoking
FcConfigDestroy() is not safe. As a result, poppler does not
invoke FcConfigDestroy() at all, although poppler frees an
object including the reference to FcConfig obtained by
FcConfigGetCurrent(). So, valgrind finds a memory leak in
some programs. See recent discussion in poppler mail list:

http://lists.freedesktop.org/archives/poppler/2010-January/005429.html
http://lists.freedesktop.org/archives/poppler/2010-January/005446.html
http://lists.freedesktop.org/archives/poppler/2010-January/005453.html

FcConfigReference() looks slightly better about its handling
of reference counter. Invoking FcConfigDestroy() for the
object obtained by FcConfigReference( NULL ) is safer than
the object obtained by FcConfigGetCurrent(). However, by P2,
the minimum reference counter of the object obtained by
FcConfigGetCurrent( NULL ) is 2, not 1. FcConfigGetCurrent()
sets the reference counter to 1, and FcConfigReference()
increments it. As a result,
  - Invoke FcConfigReference( NULL ) once
  - Invoke FcConfigDestroy( config ) twice
is required for real freeing of the FcConfig object. This is
not intuitive.

In summary, I want to propose following modifications:

M1) FcConfigGetCurrent() increments the reference counter
    when it returns existing object.

M2) FcConfigDestroy() does not increment the reference
    counter when it is invoked with NULL pointer and
    FcConfigGetCurrent() is invoked.

Please give me comment.

Regards,
mpsuzuki