memory leaks

[Marcus Brinkmann]

Hi Dirk,

On Sun, 2006-01-08 at 14:33 +0100, Dirk Stoecker wrote:
> as promised, I released the memory leak patch for gcrypt on a webpage:
> 
> http://www.dstoecker.de/gcrypt.html
> 
> It includes the motivation for the patch, the way to download and apply it 
> as well as compatibility considerations (and a not for usage in dynamic 
> loading contexts).

Your website expresses a lot of frustration.  That is understandable,
but I think we should try to approach this issue constructively.  To do
so, we have to solve a complicated technical problem, given certain
policy constraints.  If we can successfully identify these constraints,
and potential solutions, we have won a lot.  At least we will better
understand why certain changes are rejected, and if we can develop a
long term strategy.

It is very important that we untangle unrelated issues.  Your patch
contains different types of changes intermixed.  One thing you can do to
push forward this issue is to split your patch into several different
patches so we can address them individually.

One such issue is finalization.  It seems to me that your code does not
handle finalization correctly.  Consider the following case: library bar
and baz both use libgcrypt.  They both hide this fact from the
application.  The application uses the library specific initializers
(bar_init and baz_init) which each initialize libgcrypt.  Under certain
conditions this is perfectly legit, in fact, it is the only way
libgrcypt can be used correctly in such a situation.  Now, imagine baz
comes with a finalizer, which calls the finalizer for gcrypt you added.
It seems to me from skimming over your patch that in this case gcrypt is
deinitialized.  This will potentially break bar's use of gcrypt (I did
not check if it in fact does so).  It thus seems to me that your
proposed solution in its current form is at best incomplete, and I don't
see a trivial fix.

With regards to the current libgcrypt code, one overriding concern is
ABI/API compatibility.  Irregardless of what we would _want_ the code to
look like, the current libgcrypt version is used in dozens of important
software packages, and making incompatible changes to the ABI/API to the
_current_ version would need to be carefully justified.  We feel that
extension of the applicability of the library to new use cases (like
dlopen'ed plugins) does not justify breaking existing use in dozens of
software packages.  So any major changes, and my suspicion is that
cleaning up the allocation issues involves major changes if it is to be
done right, would have to go into a major revision of the whole package
including the ABI/API.  This however raises a tail of other questions,
like how do we carry over the confidence (security wise) that we have in
the current implementation to the hypothetical major revision, how can
its development be supported, etc?

Equally important would be to find a convincing story how such a major
revision could actually look like.  Moritz does have some ideas about
how to restructure the code to make it more modular and less dependent
on global statics.  However, some of the core issues are unresolved.  We
have some ideas, but I don't think we have actually found the best
solution yet, and I am not sure what the best solution would look like.

So here is how I suggest we proceed:  If you can identify separate
issues in your patch, please report them as separate issues, so they can
be discussed separately.  In particular, I want to understand if your
finalizer actually correctly solves the problem you identified (see
above).  Furthermore, I would like to know if you see ways to improve
the library in its current form without modifying the ABI/API.  Last but
not least, we can try to envision how an "ideal" libgcrypt would look
like and see if we can find mechanisms that actually allow to implement
it.  This list of issues is losely sorted from easy to hard, and from
high priority to low priority (from my perspective).

Thanks,
Marcus