Re: [PATCH] Fix memory leak in dlerror.c

[Ben Boeckel <mathstuf at gmail dot com>]

Ulrich Drepper <drepper@redhat.com> wrote:
> You have to explain what you are trying to solve.  Without looking into
> this too deeply I'd say it's unnecessary.  The error strings are not
> tied to the loading/unloading.  When a new string is needed the old one
> get freed.  I don't think any other condition is needed.  It might
> indeed be harmful if the string is still in use.
> 
> Provide a self-contained test case which shows a problem.

There is a test case attached to the Red Hat Bugzilla bug I linked to
(attached here for completeness). It only occurs when pthreads is linked
which causes the thread-specific result to be initialized and then it is
not free'd upon unloading. I think the case which gets hit is when a new
string is needed and then the library is unloaded in which case the
memory allocated is leaked.

The fact that free_key_mem is documented but never called is the real
reason I think this patch is (in essence) valid. The check-for-NULL may
be better somewhere else, but that's a technical detail.

I think that the only case when the memory would still be in use would
be when manually unloading the library in which case, I would think, you
would have to manually be aware that you may not use memory libdl may
have allocated (last_result gets free'd so any pointers to that floating
around aren't valid any more; why special-case pthreads-using
applications?). Furthermore, special casing that I have to manually the
last string libdl used isn't viable IMHO. Is the thread-local storage
even accessible outside libdl without holding a pointer that was
returned somewhere?

--Ben

Attachment: dlerror_memleak.c
Description: Text document

Attachment: dl-memleak
Description: Text document

Attachment: pgp00000.pgp
Description: PGP signature