Re: revised patch for glib compilation

[Tim Janik <timj gtk org>]

On Tue, 20 Mar 2001, Gary V. Vaughan wrote:

> Hi Tim,
> 
> Thanks for the feedback.  I have addressed all but one of your comments in 
> the attached files -- if you could help me to understand how to resolve the 
> remaining one, I'll do that and attach the result to bugzilla.  [[Aside: 
> assuming it may take a number of months before this stuff arrives in CVS, I'm 
> guessing that directly attaching the files is preferable to a large 
> bitrotting patch file?]]

it's easier for me to have the patch included in the mail text actually,
that way i can just hit Reply and insert comments, with attachments i need
to save+load those and add quote chars ;)

> > > static	inline gboolean	g_module_init_once	(void);
> > > static	inline void	g_module_set_error	(const gchar *error);
> > > static	const gchar    *g_module_get_error	(void);
> > > static	void		g_module_mutex_lock	(void);
> > > static	void		g_module_mutex_unlock	(void);
> > >
> > > /* --- variables --- */
> > > G_LOCK_DEFINE_STATIC (GModule);
> >
> > this is a recursive mutex in our current implementation, because modules
> > might want to load other modules from their g_module_check_init()
> > functions.
> 
> This is the outstanding issue that I don't understand.  I can see the need 
> for a recursive lock, but I have no idea of how to express that in terms of 
> the macros in CVS HEAD gthread.h.  Is there an example of recursive G_LOCKing 
> on the `net I can look at?

well, the best is probably gmodule/gmodule.c ;)
it's really just:

static GStaticRecMutex g_module_global_lock = G_STATIC_REC_MUTEX_INIT;

  g_static_rec_mutex_lock (&g_module_global_lock);
  g_static_rec_mutex_lock (&g_module_global_lock);
  
  g_static_rec_mutex_unlock (&g_module_global_lock);
  g_static_rec_mutex_unlock (&g_module_global_lock);

of course locking an already locked recursive lock only works from within
the thread that initially acquired the lock, all others are blocked until
the lock is released as often as it was acquired ;)

> > >       /* Make sure libltdl is using our memory managment API. */
> > >       lt_dlmalloc = (lt_ptr(*)(size_t)) g_malloc;
> > >       lt_dlfree	  = (void  (*)(lt_ptr)) g_free;
> >
> > is this required? it's safe to use malloc/free besides the glib API,
> > as long as things aren't mixed ala malloc/g_free or g_malloc/free.
> 
> It is not a requirement, but it allows G_MEM_PROFILING to see what libltdl 
> did with your memory (for example).  If you find it onerous, I can delete 
> those lines.

i wouldn't mind this memory not showing up in mem-profile, however i don't
have a strong opinion either way.

> > > GModule*
> > > g_module_open (const gchar    *file_name,
> > > 	       GModuleFlags    flags)
> >
> > you do the loading of the module and it's init call not within
> > a common lock being held, this is not going to work if two
> > threads concurrently try to load the same module, as _init
> > could be called twice here.
> 
> Good call.  Fixed.
> 
> > also you don't do any reference counting in GModule itself
> > anymore which is a problem wrg track-keeping of the init function.
> 
> libltdl keeps track of the reference count for me.  I can take a look at the 
> current count with:
> 
>     lt_dlinfo *info       = lt_dlgetinfo (handle);
>     gint       ref_count  = info->ref_count;  

yes you can, no doubt about that, but you still need to do the
decrement somewhere ;)

> > >   info = lt_dlgetinfo ((lt_dlhandle) module);
> > >
> > >   g_return_val_if_fail (info->ref_count > 1, FALSE);
> > >
> > >   if ((info->ref_count == 1) && !lt_dlisresident ((lt_dlhandle) module))
> >
> > eh? i never see you doing a decrement.
> 
> No need, I'm looking at the ref_count maintained for me by libltdl.

i.e. a program doing:

g_module_open ("foo");
h = g_module_open ("foo");
g_module_close (h);
g_module_close (h);

has this effect:

first     g_module_open ("foo"): load module into memory;
                                 if (g_module_init() != NULL)
                                   throw module out of mem;
                                 else
                                   module("foo")->ref_count = 1
second    g_module_open ("foo"): module("foo")->ref_count += 1;
first     g_module_close (h)   : module("foo")->ref_count -= 1;
second    g_module_close (h)   : module("foo")->ref_count -= 1;
                                 if (ref_count == 0 && !resident)
                                   g_module_unload();
                                 if (!resident)
                                   throw module out of memory;
so open() after a successfull open() acts as ref_count+=1 and close()
simply acts as ref_count-=1 untill 0 is reached.
when 0 is we do the unload call and actually unload the module if
it wasn't made resident. not the extra check for residency after
g_module_unload(), because that funciton may still make the
module resident.

> > [[snip]]
> > (another implication just sprang to my mind, GModule doesn't currently
> > handle static C++ constructors
> 
> Nor does libltdl, since lt_dlsym() doesn't really work with the C++ name 
> mangler.  Unfortunately, this means that the dlopen concept doesn't really 
> port to C++.  I can imagine a hairy workaround, but I don't inflict C++ on 
> myself outside of work *grin* =)O|

oh, i thought someone had implemented the code in libltdl to get static
c++ constructors to work. anyways, in the case such code arrives, it may only
be called after successfull g_module_init(). that means for libtool, there
must be ways to supress automatic constructor initialization at loading time
and do them on demand later on.

> > > gchar*
> > > g_module_build_path (const gchar *directory,
> > > 		     const gchar *module_name)
> > > {
> >
> > how's libtool going to handle "libfoo.la" vs. "libfoo.so" vs. "libfoo.dll"
> > vs. "libfoo" vs. "foo" arguments being passed into open()?
> 
> lt_dlopenext("libfoo") (as used by g_module_open()) adds the host platform's 
> module extension, if dlopening of the exact argument string failed.  So, 
> rather than:
> 
> 	g_module_open (g_module_build_path (NULL, "libfoo"));
> 
> You would use:
> 
> 	g_module_open ("libfoo");
> 
> and libltdl takes care of the rest (although both work, since the literal 
> string is tried first).

well, the current build path code adds "lib" prefixes on demand as well...

> 
> Cheers,
> 	Gary.

---
ciaoTJ