On Wed, Jan 04, 2017 at 01:22:03AM -0500, Rich Felker wrote:
> On Wed, Jan 04, 2017 at 01:06:40AM -0500, Rich Felker wrote:
> > diff --git a/ldso/dynlink.c b/ldso/dynlink.c
> > index c689084..cb82b2c 100644
> > --- a/ldso/dynlink.c
> > +++ b/ldso/dynlink.c
> > @@ -67,6 +67,7 @@ struct dso {
> >  	char constructed;
> >  	char kernel_mapped;
> >  	struct dso **deps, *needed_by;
> > +	size_t next_dep;
> >  	char *rpath_orig, *rpath;
> >  	struct tls_module tls;
> >  	size_t tls_id;
> > @@ -1211,13 +1212,14 @@ void __libc_exit_fini()
> >  static void do_init_fini(struct dso *p)
> >  {
> >  	size_t dyn[DYN_CNT];
> > -	int need_locking = libc.threads_minus_1;
> > -	/* Allow recursive calls that arise when a library calls
> > -	 * dlopen from one of its constructors, but block any
> > -	 * other threads until all ctors have finished. */
> > -	if (need_locking) pthread_mutex_lock(&init_fini_lock);
> > -	for (; p; p=p->prev) {
> > -		if (p->constructed) continue;
> > +	pthread_mutex_lock(&init_fini_lock);
> > +	while (!p->constructed) {
> > +		while (p->deps[p->next_dep] && p->deps[p->next_dep]->constructed)
> > +			p->next_dep++;
> > +		if (p->deps[p->next_dep]) {
> > +			p = p->deps[p->next_dep++];
> > +			continue;
> > +		}
> 
> I think this logic is probably broken in the case of circular
> dependencies, and will end up skipping ctors for some deps due to the
> increment in this line:
> 
> +			p = p->deps[p->next_dep++];
> 
> which happens before the new p's ctors have actually run. Omitting the
> increment here, however, would turn it into an infinite loop. We need
> some way to detect this case and let the dso with an indirect
> dependency on itself run its ctors once all non-circular deps have
> been satisfied. I don't now the right algorithm for this right off,
> though; suggestions would be welcome.

Here's a v2 of the patch with the above issues fixed, and some
comments that hopefully make it make sense. I still think there's more
logic needed to allow concurrent ctors from unrelated dlopen in
multiple threads, though.

Rich