Re: ltdl bugs

[Ralf Wildenhues]

* Jeff Squyres wrote on Sun, Apr 03, 2005 at 04:54:32PM CEST:
> On Apr 3, 2005, at 10:17 AM, Ralf Wildenhues wrote:
> 
> >>I tried moving the three LT_DLFREE after the brace, and it /seems/ ok
> >>then.  Am quite reluctant to put it in yet, because I fear to have
> >>overlooked something.
> 
> So actually, I didn't do that for a specific reason: they're NULL in 
> the self case.

Sure.  But ltdl allows you to do

   handle = lt_dlopen("foo.la");
   lt_dlmakeresident(handle);
   lt_dlclose(handle);

and also (before the close)
   lt_dlcaller_id key = lt_dlcaller_register();
   lt_ptr p = lt_dlcaller_set_data (key, handle, f);

Then the other pointers are not NULL.  Read
  info libtool 'User defined module data'
on how to free such data.

> Granted -- I only tested this on Linux; it's quite possible that 
> they're not NULL in other cases.   Easy to move these FREE's beyond the 
> brace and protect them in "if" statements.

Erm, no need to do that.  LT_DLFREE checks for NULL, that's one reason
we have the macro.

> >Ah, yes, here's the argument:
> >
> >When we accept your patch, Jeff, we allow to let ltdl's view of loaded
> >modules get out of sync with what is really loaded.  Now promise me 
> >that
> >we don't currently use any system API which disallows opening of an
> >already opened module.  Nor that we will ever implement support for 
> >such
> >an API.

> >I don't feel competent to judge on this assumption.
> >
> >Note it might be possible to do what you want inside lt_dlexit(), but
> >the amount of hair would grow.
> 
> Hmm.  I'm not sure I understand this.  Doesn't "resident" mean that the 
> module is statically compiled in the application (i.e., no underlying 
> dlopen() [or whatever] was required to load it)?  If so:

No.  It /can/ mean that, but it can also mean that the application
writer does not want this module to be unloadable.  She may just call
lt_dlmakeresident() to enforce this.

> - we're talking about the pre-sym case:

Not necessarily.

> presym_close() is a no-op, so 
> it is safe to call this function, even in the self or pre-loaded case.

Yes, for preload this is fine.

> - we're also talking about the self case:

Not necessarily.

> on Linux, that's 
> sys_dl_open(), which calls dlopen() with a filename==NULL (and gets a 
> handle back).  Technically, we should call dlclose() with that handle, 
> too -- but the man page (on Linux) doesn't specify what happens in that 
> scenario.  Some tests on Linux and Solaris seem to show that this is ok 
> (which makes sense -- if dlopen() allows you to open NULL, then it must 
> allow you to close it, too).  So it is safe to call this function for 
> the self case.

Yes, in these special cases.  For the libc versions you tested.

> - also, ltdl's view of loaded modules is out of sync with what is 
> really loaded until you call lt_dlopen() the first time.

You are not guaranteed anything else, sure.  Maybe we should explicitly
add to the docs:  "If another part of your application uses a module
loading interface (such as dlopen()) without wrapping it through
libltdl, then the behavior is undefined."

> So my [ammended, per above] patch simply resets ltdl's view of what is 
> loaded to what it was before you called lt_dlopen().
> 
> Is that correct?  If so, my patch actually becomes much simpler -- 
> there's no need to cordon off the unload module stuff -- just protect 
> the DLFREE's with if statements:

No, that is not correct.  The if() are redundant, however, for reasons
stated above.

> Or I'm missing something...?

Yes.

I'll try to rephrase:  You may actually do what you intend in
lt_dlexit(), because that is (like MPI_Finalize) final in the sense,
that after as many lt_dlexit's as lt_dlinit's we may completely shut
down our stuff and free every data we own.

You may _not_ do this in lt_dlclose(), however, because the user may
issue another lt_dlopen() of the very same module (which may be NULL, or
"foo.la" made resident), which in fact is already open.

Combine that with the fact that I simply don't know what will happen
then for any of the system-specific module interfaces we implement.
Thus, I regard this as disallowed.

Could I make myself clear now?

Regards,
Ralf