ldso: dlclose.

ldso: dlclose.

[musl]

Hi,

I noticed that the dlclose function is not implemented (always return 0).
It might be a problem in my case cause I use dlfuncs to implement a plugin system and when I unload a plugin, the memory
is not released.

Do you plan to implement this function ?

I can work on this but it seems that __cxa_finalize is not implemented either and the 'struct fl' (atexit.c) does not
contain any dso field.
Do you have some concern about how this should be done.

BTW I think there is a bug in the '__funcs_on_exit' (atexit.c) : only the first non null function of each function pool
is called.


Something like this should do the trick.

void __funcs_on_exit()
{
    int i;
    void (*func)(void *), *arg;
    LOCK(lock);
    for (; head; head=head->next) {
        for (i=COUNT-1; i>=0 && !head->f[i]; i--);
        if (i<0) continue;
        for (; i>=0; i--) {
            func = head->f[i];
            arg = head->a[i];
            head->f[i] = 0;
            UNLOCK(lock);
            func(arg);
            LOCK(lock);
        }
    }
}

Regards,

Boris

Re: ldso: dlclose.

[Rich Felker]

On Sun, Aug 19, 2012 at 06:26:45PM +0200, musl wrote:
> Hi,
> 
> I noticed that the dlclose function is not implemented (always return 0).
> It might be a problem in my case cause I use dlfuncs to implement a plugin system and when I unload a plugin, the memory
> is not released.
> 
> Do you plan to implement this function ?

It is implemented. Per POSIX:

    The use of dlclose() reflects a statement of intent on the part of
    the process, but does not create any requirement upon the
    implementation, such as removal of the code or symbols referenced
    by handle.

Source:
http://pubs.opengroup.org/onlinepubs/9699919799/functions/dlclose.html

To elaborate, removing a DSO from a process's address space is highly
non-trivial and error-prone. If you've spent any time reading glibc's
bug tracker, you'll find they've had a lot of nasty bugs stemming from
dlclose doing the wrong things. I don't want to recreate that kind of
bugginess. The requirements get even worse once we implement TLS.

At first, I thought it might be feasible to just support unloading of
DOSs that were loaded non-global, since there's no way their symbols
have been used to resolve references elsewhere in the program. However
there are still major ugly issues that seemingly can't be handled
correctly. Suppose for instance myplugin.so is part of your
application and intended only to be loaded/unloaded with
dlopen/dlclose, never linked directly, but it depends on libfoo.so
which was designed to be linked directly. Both will get loaded
non-globally when dlopen is called on myplugin.so, but libfoo.so might
do things that are incompatible with later being unloaded -- for
example, registering an atexit function. You can try to solve this
issue with __cxa_atexit and DSO handles but that only covers a special
case. The library might instead have registered its atexit function
through code in a third library, or it might not even be directly
registering an atexit function but instead a cleanup function that
some other library will call directly during its own cleanup routines,
or even registered pointers to functions or data within libfoo.so with
an interface in another library that's not doing to be unloaded at the
same time.

What it comes down to is that it's never safe to unload a library that
was not designed to be unloadable. As such, the maximum possible
unloading we could ever safely support is unloading only the dlopen'd
library itself and never any of its dependencies; but even that's not
entirely safe since some programs will directly dlopen libraries that
were not designed to be loaded and unloaded.

Also, note that all of the above is purely the fundamental issues that
make unloading a bad idea; I haven't even touched on all the things
that are easy to get wrong in the implementation, leading to race
conditions and crashes in the dynamic linker itself.

In summary, I don't see any way to make a correct implementation where
dlclose actually unloads stuff. If you have ideas for how to do it,
please explain; I'm not against doing it, but only if it can be done
correctly.

> BTW I think there is a bug in the '__funcs_on_exit' (atexit.c) : only the first non null function of each function pool
> is called.

I'll take a look...

> Something like this should do the trick.
> 
> void __funcs_on_exit()
> {
>     int i;
>     void (*func)(void *), *arg;
>     LOCK(lock);
>     for (; head; head=head->next) {
>         for (i=COUNT-1; i>=0 && !head->f[i]; i--);
>         if (i<0) continue;
>         for (; i>=0; i--) {
>             func = head->f[i];
>             arg = head->a[i];
>             head->f[i] = 0;
>             UNLOCK(lock);
>             func(arg);
>             LOCK(lock);
>         }
>     }
> }

You're right. Thanks for the catch! I've fixed it slightly differently
(avoiding two separate loops over the array).

Rich

Re: ldso: dlclose.

[Rich Felker]

On Sun, Aug 19, 2012 at 08:48:03PM -0400, Rich Felker wrote:
> In summary, I don't see any way to make a correct implementation where
> dlclose actually unloads stuff. If you have ideas for how to do it,
> please explain; I'm not against doing it, but only if it can be done
> correctly.

And for completeness' sake, I posted a bug to the glibc bug tracker:

http://sourceware.org/bugzilla/show_bug.cgi?id=14511

Rich

Re: ldso: dlclose.

[Arvid E. Picciani]

On Sun, 19 Aug 2012 20:48:03 -0400, Rich Felker wrote:

> To elaborate, removing a DSO from a process's address space is highly
> non-trivial and error-prone.

In fact it doesn't even work correctly in glibc.
It's just that usually no one notices.

When i see people using plugin architectures with unload,
it's almost always coming down from a  high level design.

The lack of any usable IPC in linux lead to glueing the
components into one process.
Plugins make it appear clean, but really it's not clean at all.
Much worse when C++ comes into the mix and arbitrary
things happen at the "abi" level.

Plugins are _not_ a good component separation design.
Neither are any of the alternatives, of course...

Another sad story of how the religious view of one kernel
maintainer creates a whole universe of bad design further down.



-- 
Arvid E. Picciani

Re: ldso: dlclose.

[Rich Felker]

On Thu, Aug 23, 2012 at 02:39:38PM +0200, Arvid E. Picciani wrote:
> On Sun, 19 Aug 2012 20:48:03 -0400, Rich Felker wrote:
> 
> >To elaborate, removing a DSO from a process's address space is highly
> >non-trivial and error-prone.
> 
> In fact it doesn't even work correctly in glibc.
> It's just that usually no one notices.

Indeed, and they closed my bug report about it as invalid. I've
refiled the bug with binutils, which has the -z nodelete option,
requesting that this option be made default. Only libraries
intentionally marked as unload-safe should be unloadable.

Anyway, unless the issue is fixed in binutils so that the vast
majority of libraries are marked non-unloadable, I don't see anything
we can do in musl. "glibc does it that way too" is not an excuse for
adding unsafe/non-robust behavior to musl.

Rich

Re: ldso: dlclose.

[orc]

On Thu, 23 Aug 2012 08:48:16 -0400
Rich Felker <dalias@aerifal.cx> wrote:

> Anyway, unless the issue is fixed in binutils so that the vast
> majority of libraries are marked non-unloadable, I don't see anything
> we can do in musl. "glibc does it that way too" is not an excuse for
> adding unsafe/non-robust behavior to musl.
> 
> Rich

The whole dlopen/dlclose/dlsym functions family are 'harmful': even if
we want static linking, application will still rely on them and fail
invisibly, creating more headaches.
I think better leave dlclose() in it's current state now. It will always
'success', nobody will care.

Re: ldso: dlclose.

[Rich Felker]

On Fri, Aug 24, 2012 at 12:02:09AM +0800, orc wrote:
> On Thu, 23 Aug 2012 08:48:16 -0400
> Rich Felker <dalias@aerifal.cx> wrote:
> 
> > Anyway, unless the issue is fixed in binutils so that the vast
> > majority of libraries are marked non-unloadable, I don't see anything
> > we can do in musl. "glibc does it that way too" is not an excuse for
> > adding unsafe/non-robust behavior to musl.
> > 
> > Rich
> 
> The whole dlopen/dlclose/dlsym functions family are 'harmful': even if
> we want static linking, application will still rely on them and fail
> invisibly, creating more headaches.
> I think better leave dlclose() in it's current state now. It will always
> 'success', nobody will care.

In my view, there are only two downsides to the current behavior:

1. Some buggy plugin-based applications may expect dlclose(plugin) to
call the destructors in the plugin. This is of course an invalid
expectation per POSIX, but it may be the reality for some apps.

2. In an extremely long-lived app that loads and unloads plugins which
may be upgraded multiple times during the application's lifetime, each
new version of the plugin will consume additional virtual memory space
and commit charge, i.e. you have a memory leak. In the real world the
leak should be very slow, but it could become significant if the
plugins are very large and get reinstalled many times, perhaps if
someone is experimenting and running "make install" each time...

In my view #2 is a very low-priority problem that's not worth caring
about on its own, but #1 may be relevant. If does become an important
issue that we can't get fixed at the application level, I think the
solution would be to add unloading, but have it only take effect for
the actual argument to dlopen/dlclose, never any libraries implicitly
loaded as dependencies (and of course to honor the flag that prevents
unloading).

Rich

Re: ldso: dlclose.

[musl]

On 23/08/2012 20:01, Rich Felker wrote:
> On Fri, Aug 24, 2012 at 12:02:09AM +0800, orc wrote:
>> On Thu, 23 Aug 2012 08:48:16 -0400
>> Rich Felker <dalias@aerifal.cx> wrote:
>>
>>> Anyway, unless the issue is fixed in binutils so that the vast
>>> majority of libraries are marked non-unloadable, I don't see anything
>>> we can do in musl. "glibc does it that way too" is not an excuse for
>>> adding unsafe/non-robust behavior to musl.
>>>
>>> Rich
>> The whole dlopen/dlclose/dlsym functions family are 'harmful': even if
>> we want static linking, application will still rely on them and fail
>> invisibly, creating more headaches.
>> I think better leave dlclose() in it's current state now. It will always
>> 'success', nobody will care.
> In my view, there are only two downsides to the current behavior:
>
> 1. Some buggy plugin-based applications may expect dlclose(plugin) to
> call the destructors in the plugin. This is of course an invalid
> expectation per POSIX, but it may be the reality for some apps.
Indeed, many plugins implem rely on constructors/destructors to allocate/free memory or intialize/cleanup context.
This may lead to memory leaks or other issues if the plugin is loaded/unloaded multiple times.
>
> 2. In an extremely long-lived app that loads and unloads plugins which
> may be upgraded multiple times during the application's lifetime, each
> new version of the plugin will consume additional virtual memory space
> and commit charge, i.e. you have a memory leak. In the real world the
> leak should be very slow, but it could become significant if the
> plugins are very large and get reinstalled many times, perhaps if
> someone is experimenting and running "make install" each time...
It might be worst for long-lived apps running in a memory constrained environment (embedded systems).
>
> In my view #2 is a very low-priority problem that's not worth caring
> about on its own, but #1 may be relevant. If does become an important
> issue that we can't get fixed at the application level, I think the
> solution would be to add unloading, but have it only take effect for
> the actual argument to dlopen/dlclose, never any libraries implicitly
> loaded as dependencies (and of course to honor the flag that prevents
> unloading).
Does this mean you want to call plugin destructors in dlclose function and keep the plugin memory mapping ?
>
> Rich

Re: ldso: dlclose.

[Rich Felker]

On Fri, Aug 24, 2012 at 09:52:28AM +0200, musl wrote:
> On 23/08/2012 20:01, Rich Felker wrote:
> > On Fri, Aug 24, 2012 at 12:02:09AM +0800, orc wrote:
> >> On Thu, 23 Aug 2012 08:48:16 -0400
> >> Rich Felker <dalias@aerifal.cx> wrote:
> >>
> >>> Anyway, unless the issue is fixed in binutils so that the vast
> >>> majority of libraries are marked non-unloadable, I don't see anything
> >>> we can do in musl. "glibc does it that way too" is not an excuse for
> >>> adding unsafe/non-robust behavior to musl.
> >>>
> >>> Rich
> >> The whole dlopen/dlclose/dlsym functions family are 'harmful': even if
> >> we want static linking, application will still rely on them and fail
> >> invisibly, creating more headaches.
> >> I think better leave dlclose() in it's current state now. It will always
> >> 'success', nobody will care.
> > In my view, there are only two downsides to the current behavior:
> >
> > 1. Some buggy plugin-based applications may expect dlclose(plugin) to
> > call the destructors in the plugin. This is of course an invalid
> > expectation per POSIX, but it may be the reality for some apps.
> Indeed, many plugins implem rely on constructors/destructors to
> allocate/free memory or intialize/cleanup context.
> This may lead to memory leaks or other issues if the plugin is
> loaded/unloaded multiple times.

A plugin cannot be loaded more than once. Subsequent calls to dlopen
use the existing loaded image. The only way it could be loaded again
is if the file were replaced by a new version.

I think maybe you're not realizing that the "leak" can only happen if
a new version of the .so file is put in place of the old one...

> > 2. In an extremely long-lived app that loads and unloads plugins which
> > may be upgraded multiple times during the application's lifetime, each
> > new version of the plugin will consume additional virtual memory space
> > and commit charge, i.e. you have a memory leak. In the real world the
> > leak should be very slow, but it could become significant if the
> > plugins are very large and get reinstalled many times, perhaps if
> > someone is experimenting and running "make install" each time...
> It might be worst for long-lived apps running in a memory
> constrained environment (embedded systems).

Yes, but in this kind of system, ANY use of dynamic memory allocation
is frowned upon. Dynamic module loading even moreso. And of course I
don't think you'll be constantly replacing .so files on such a system
with new versions.

> > In my view #2 is a very low-priority problem that's not worth caring
> > about on its own, but #1 may be relevant. If does become an important
> > issue that we can't get fixed at the application level, I think the
> > solution would be to add unloading, but have it only take effect for
> > the actual argument to dlopen/dlclose, never any libraries implicitly
> > loaded as dependencies (and of course to honor the flag that prevents
> > unloading).
> Does this mean you want to call plugin destructors in dlclose
> function and keep the plugin memory mapping ?

No. Calling dtors and unloading always come in a pair. You cannot call
dtors but keep and reuse the mapping because the static-storage
objects would retain their old values from the prior load, but a new
load would be visible to the code in the plugin.

The potential design I'm talking about would have only the dlopen'd
library itself ever unloaded/unmapped. For example, if myplugin.so
depends on libfoo.so and libbar.so, libfoo.so and libbar.so, which
were implicitly loaded when loading myplugin.sh, will never be
unmappable. Only myplugin.so itself would be unmappable. On
unloading/unmapping dtors would be called as usual, and then the
reference would be removed entirely from the DSO chain, causing it to
be searched-out and loaded new next time dlopen is called.

I do not want to do this except as a last resort, since as I've
already mentioned it's highly error-prone (see glibc) and fragile.

Rich

Re: ldso: dlclose.

[musl]

On 24/08/2012 14:27, Rich Felker wrote:
> On Fri, Aug 24, 2012 at 09:52:28AM +0200, musl wrote:
>> On 23/08/2012 20:01, Rich Felker wrote:
>>> On Fri, Aug 24, 2012 at 12:02:09AM +0800, orc wrote:
>>>> On Thu, 23 Aug 2012 08:48:16 -0400
>>>> Rich Felker <dalias@aerifal.cx> wrote:
>>>>
>>>>> Anyway, unless the issue is fixed in binutils so that the vast
>>>>> majority of libraries are marked non-unloadable, I don't see anything
>>>>> we can do in musl. "glibc does it that way too" is not an excuse for
>>>>> adding unsafe/non-robust behavior to musl.
>>>>>
>>>>> Rich
>>>> The whole dlopen/dlclose/dlsym functions family are 'harmful': even if
>>>> we want static linking, application will still rely on them and fail
>>>> invisibly, creating more headaches.
>>>> I think better leave dlclose() in it's current state now. It will always
>>>> 'success', nobody will care.
>>> In my view, there are only two downsides to the current behavior:
>>>
>>> 1. Some buggy plugin-based applications may expect dlclose(plugin) to
>>> call the destructors in the plugin. This is of course an invalid
>>> expectation per POSIX, but it may be the reality for some apps.
>> Indeed, many plugins implem rely on constructors/destructors to
>> allocate/free memory or intialize/cleanup context.
>> This may lead to memory leaks or other issues if the plugin is
>> loaded/unloaded multiple times.
> A plugin cannot be loaded more than once. Subsequent calls to dlopen
> use the existing loaded image. The only way it could be loaded again
> is if the file were replaced by a new version.
>
> I think maybe you're not realizing that the "leak" can only happen if
> a new version of the .so file is put in place of the old one...
I was talking about this specific case :
1) unloding a plugin
2) updating the plugin (new plugin.so)
3) reloading the plugin

During the whole sequence the application is up and running.

Here is how I should do it if dlclose is implemented per posix :
1) stop the application
2) update the plugin
3) restart the application

The application is not available during this sequence.

>
>>> 2. In an extremely long-lived app that loads and unloads plugins which
>>> may be upgraded multiple times during the application's lifetime, each
>>> new version of the plugin will consume additional virtual memory space
>>> and commit charge, i.e. you have a memory leak. In the real world the
>>> leak should be very slow, but it could become significant if the
>>> plugins are very large and get reinstalled many times, perhaps if
>>> someone is experimenting and running "make install" each time...
>> It might be worst for long-lived apps running in a memory
>> constrained environment (embedded systems).
> Yes, but in this kind of system, ANY use of dynamic memory allocation
> is frowned upon. Dynamic module loading even moreso. And of course I
> don't think you'll be constantly replacing .so files on such a system
> with new versions.
>
>>> In my view #2 is a very low-priority problem that's not worth caring
>>> about on its own, but #1 may be relevant. If does become an important
>>> issue that we can't get fixed at the application level, I think the
>>> solution would be to add unloading, but have it only take effect for
>>> the actual argument to dlopen/dlclose, never any libraries implicitly
>>> loaded as dependencies (and of course to honor the flag that prevents
>>> unloading).
>> Does this mean you want to call plugin destructors in dlclose
>> function and keep the plugin memory mapping ?
> No. Calling dtors and unloading always come in a pair. You cannot call
> dtors but keep and reuse the mapping because the static-storage
> objects would retain their old values from the prior load, but a new
> load would be visible to the code in the plugin.
>
> The potential design I'm talking about would have only the dlopen'd
> library itself ever unloaded/unmapped. For example, if myplugin.so
> depends on libfoo.so and libbar.so, libfoo.so and libbar.so, which
> were implicitly loaded when loading myplugin.sh, will never be
> unmappable. Only myplugin.so itself would be unmappable. On
> unloading/unmapping dtors would be called as usual, and then the
> reference would be removed entirely from the DSO chain, causing it to
> be searched-out and loaded new next time dlopen is called.
>
> I do not want to do this except as a last resort, since as I've
> already mentioned it's highly error-prone (see glibc) and fragile.
I understand your concern and I'll modify my code to get rid of the dlclose function.
I hope there's no other apps or libs relying on gnu dlclose specific implem.
It should not if they've read carrefully the dlclose man page :-).

BTW, thanks for taking the time to explain the dlclose implications.
>
> Rich

Re: ldso: dlclose.

[Rich Felker]

On Fri, Aug 24, 2012 at 03:54:25PM +0200, musl wrote:
> I was talking about this specific case :
> 1) unloding a plugin
> 2) updating the plugin (new plugin.so)
> 3) reloading the plugin
> 
> During the whole sequence the application is up and running.
> 
> Here is how I should do it if dlclose is implemented per posix :
> 1) stop the application
> 2) update the plugin
> 3) restart the application

To clarify on "per POSIX", POSIX allows but does not require dlclose
on the last reference to remove the library from the process's address
space, but requires that this not happen if the library has been used
to resolve undefined symbols.

> The application is not available during this sequence.

If the application just re-execs itself rather than exiting, there is
no window during which it's unavailable. See how irssi handles the
/upgrade command for an example. That might be the cleanest way to
handle what you want to do.

Rich

Re: ldso: dlclose.

[boris brezillon]

2012/8/24 Rich Felker <dalias@aerifal.cx>:
> On Fri, Aug 24, 2012 at 03:54:25PM +0200, musl wrote:
>> I was talking about this specific case :
>> 1) unloding a plugin
>> 2) updating the plugin (new plugin.so)
>> 3) reloading the plugin
>>
>> During the whole sequence the application is up and running.
>>
>> Here is how I should do it if dlclose is implemented per posix :
>> 1) stop the application
>> 2) update the plugin
>> 3) restart the application
>
> To clarify on "per POSIX", POSIX allows but does not require dlclose
> on the last reference to remove the library from the process's address
> space, but requires that this not happen if the library has been used
> to resolve undefined symbols.
>
>> The application is not available during this sequence.
>
> If the application just re-execs itself rather than exiting, there is
> no window during which it's unavailable. See how irssi handles the
> /upgrade command for an example. That might be the cleanest way to
> handle what you want to do.
>
This sounds interesting, I'll take a look.
> Rich