Re: hang in dlclose() on rtld lock (powerpc64)

From: Justin Hibbits <jrh29_at_alumni.cwru.edu>
Date: Thu, 9 Mar 2017 14:36:19 -0600
On Thursday, March 9, 2017, Konstantin Belousov <kostikbel_at_gmail.com> wrote:

> On Thu, Mar 09, 2017 at 09:59:00AM -0600, Justin Hibbits wrote:
> > When building ports in poudriere, I see gdk-pixbuf-query-modules and
> > gio-querymodules hanging on r314676, but working in r305820.  I took a
> > backtrace on both in gdb, and see the following (identical between both):
> >
> > Program received signal SIGINT, Interrupt.
> > 0x00000000506831d8 in .__sys.umtx_op () from /lib/libc.so.7
> > (gdb) bt
> > #0  0x00000000506831d8 in .__sys.umtx_op () from /lib/libc.so.7
> > #1  0x0000000050588010 in _umtx_op_err (obj=0x4, op=13, val=0, uaddr=0x0,
> >     uaddr2=0x0)
> >     at /home/chmeee/freebsd/pristine/lib/libthr/thread/thr_umtx.c:37
> > #2  0x00000000505881b8 in __thr_rwlock_wrlock (rwlock=<optimized out>,
> >     tsp=<optimized out>)
> >     at /home/chmeee/freebsd/pristine/lib/libthr/thread/thr_umtx.c:325
> > #3  0x00000000505965f0 in _thr_rwlock_wrlock (tsp=<optimized out>,
> >     rwlock=<optimized out>)
> >     at /home/chmeee/freebsd/pristine/lib/libthr/thread/thr_umtx.h:239
> > #4  _thr_rtld_wlock_acquire (lock=0x505bdd00)
> >     at /home/chmeee/freebsd/pristine/lib/libthr/thread/thr_rtld.c:141
> > #5  0x0000000050026bf4 in wlock_acquire (lock=0x5004cf20 <rtld_locks>,
> >     lockstate=0xffffffffffffcab0)
> >     at /home/chmeee/freebsd/pristine/libexec/rtld-elf/rtld_lock.c:222
> > #6  0x0000000050022b1c in dlclose (handle=0x51d62000)
> >     at /home/chmeee/freebsd/pristine/libexec/rtld-elf/rtld.c:3021
> > #7  0x0000000050022c90 in free_needed_filtees (n=0x509f7420)
> >     at /home/chmeee/freebsd/pristine/libexec/rtld-elf/rtld.c:2113
> > #8  0x0000000050022d18 in unload_filtees (obj=0x509fa800)
> >     at /home/chmeee/freebsd/pristine/libexec/rtld-elf/rtld.c:2129
> > #9  0x0000000050022e54 in unload_object (root=<optimized out>)
> >     at /home/chmeee/freebsd/pristine/libexec/rtld-elf/rtld.c:4464
> > #10 0x0000000050022c20 in dlclose (handle=0x50054000)
> >     at /home/chmeee/freebsd/pristine/libexec/rtld-elf/rtld.c:3044
> > ---Type <return> to continue, or q <return> to quit---q
> >
> >
> > This happens on powerpc64.  I haven't tested on powerpc or any other
> arch.
>
> Please test the following patch.  It avoids recursing on the bind lock.
>
> diff --git a/libexec/rtld-elf/rtld.c b/libexec/rtld-elf/rtld.c
> index a7c61b2d13f..880cf100c45 100644
> --- a/libexec/rtld-elf/rtld.c
> +++ b/libexec/rtld-elf/rtld.c
> _at__at_ -77,6 +77,7 _at__at_ static void digest_dynamic2(Obj_Entry *, const Elf_Dyn
> *, const Elf_Dyn *,
>  static void digest_dynamic(Obj_Entry *, int);
>  static Obj_Entry *digest_phdr(const Elf_Phdr *, int, caddr_t, const char
> *);
>  static Obj_Entry *dlcheck(void *);
> +static int dlclose_locked(void *, RtldLockState *);
>  static Obj_Entry *dlopen_object(const char *name, int fd, Obj_Entry
> *refobj,
>      int lo_flags, int mode, RtldLockState *lockstate);
>  static Obj_Entry *do_load_object(int, const char *, char *, struct stat
> *, int);
> _at__at_ -98,7 +99,7 _at__at_ static void initlist_add_objects(Obj_Entry *, Obj_Entry
> *, Objlist *);
>  static void linkmap_add(Obj_Entry *);
>  static void linkmap_delete(Obj_Entry *);
>  static void load_filtees(Obj_Entry *, int flags, RtldLockState *);
> -static void unload_filtees(Obj_Entry *);
> +static void unload_filtees(Obj_Entry *, RtldLockState *);
>  static int load_needed_objects(Obj_Entry *, int);
>  static int load_preload_objects(void);
>  static Obj_Entry *load_object(const char *, int fd, const Obj_Entry *,
> int);
> _at__at_ -142,7 +143,7 _at__at_ static int symlook_obj1_sysv(SymLook *, const
> Obj_Entry *);
>  static int symlook_obj1_gnu(SymLook *, const Obj_Entry *);
>  static void trace_loaded_objects(Obj_Entry *);
>  static void unlink_object(Obj_Entry *);
> -static void unload_object(Obj_Entry *);
> +static void unload_object(Obj_Entry *, RtldLockState *lockstate);
>  static void unref_dag(Obj_Entry *);
>  static void ref_dag(Obj_Entry *);
>  static char *origin_subst_one(Obj_Entry *, char *, const char *,
> _at__at_ -2104,13 +2105,13 _at__at_ initlist_add_objects(Obj_Entry *obj, Obj_Entry
> *tail, Objlist *list)
>  #endif
>
>  static void
> -free_needed_filtees(Needed_Entry *n)
> +free_needed_filtees(Needed_Entry *n, RtldLockState *lockstate)
>  {
>      Needed_Entry *needed, *needed1;
>
>      for (needed = n; needed != NULL; needed = needed->next) {
>         if (needed->obj != NULL) {
> -           dlclose(needed->obj);
> +           dlclose_locked(needed->obj, lockstate);
>             needed->obj = NULL;
>         }
>      }
> _at__at_ -2121,14 +2122,14 _at__at_ free_needed_filtees(Needed_Entry *n)
>  }
>
>  static void
> -unload_filtees(Obj_Entry *obj)
> +unload_filtees(Obj_Entry *obj, RtldLockState *lockstate)
>  {
>
> -    free_needed_filtees(obj->needed_filtees);
> -    obj->needed_filtees = NULL;
> -    free_needed_filtees(obj->needed_aux_filtees);
> -    obj->needed_aux_filtees = NULL;
> -    obj->filtees_loaded = false;
> +       free_needed_filtees(obj->needed_filtees, lockstate);
> +       obj->needed_filtees = NULL;
> +       free_needed_filtees(obj->needed_aux_filtees, lockstate);
> +       obj->needed_aux_filtees = NULL;
> +       obj->filtees_loaded = false;
>  }
>
>  static void
> _at__at_ -3015,15 +3016,23 _at__at_ search_library_pathfds(const char *name, const
> char *path, int *fdp)
>  int
>  dlclose(void *handle)
>  {
> +       RtldLockState lockstate;
> +       int error;
> +
> +       wlock_acquire(rtld_bind_lock, &lockstate);
> +       error = dlclose_locked(handle, &lockstate);
> +       lock_release(rtld_bind_lock, &lockstate);
> +       return (error);
> +}
> +
> +static int
> +dlclose_locked(void *handle, RtldLockState *lockstate)
> +{
>      Obj_Entry *root;
> -    RtldLockState lockstate;
>
> -    wlock_acquire(rtld_bind_lock, &lockstate);
>      root = dlcheck(handle);
> -    if (root == NULL) {
> -       lock_release(rtld_bind_lock, &lockstate);
> +    if (root == NULL)
>         return -1;
> -    }
>      LD_UTRACE(UTRACE_DLCLOSE_START, handle, NULL, 0, root->dl_refcount,
>         root->path);
>
> _at__at_ -3035,19 +3044,18 _at__at_ dlclose(void *handle)
>          * The object will be no longer referenced, so we must unload it.
>          * First, call the fini functions.
>          */
> -       objlist_call_fini(&list_fini, root, &lockstate);
> +       objlist_call_fini(&list_fini, root, lockstate);
>
>         unref_dag(root);
>
>         /* Finish cleaning up the newly-unreferenced objects. */
>         GDB_STATE(RT_DELETE,&root->linkmap);
> -       unload_object(root);
> +       unload_object(root, lockstate);
>         GDB_STATE(RT_CONSISTENT,NULL);
>      } else
>         unref_dag(root);
>
>      LD_UTRACE(UTRACE_DLCLOSE_STOP, handle, NULL, 0, 0, NULL);
> -    lock_release(rtld_bind_lock, &lockstate);
>      return 0;
>  }
>
> _at__at_ -3123,13 +3131,13 _at__at_ rtld_dlopen(const char *name, int fd, int mode)
>  }
>
>  static void
> -dlopen_cleanup(Obj_Entry *obj)
> +dlopen_cleanup(Obj_Entry *obj, RtldLockState *lockstate)
>  {
>
>         obj->dl_refcount--;
>         unref_dag(obj);
>         if (obj->refcount == 0)
> -               unload_object(obj);
> +               unload_object(obj, lockstate);
>  }
>
>  static Obj_Entry *
> _at__at_ -3178,7 +3186,7 _at__at_ dlopen_object(const char *name, int fd, Obj_Entry
> *refobj, int lo_flags,
>               (mode & RTLD_MODEMASK) == RTLD_NOW, &obj_rtld,
>               (lo_flags & RTLD_LO_EARLY) ? SYMLOOK_EARLY : 0,
>               lockstate) == -1) {
> -               dlopen_cleanup(obj);
> +               dlopen_cleanup(obj, lockstate);
>                 obj = NULL;
>             } else if (lo_flags & RTLD_LO_EARLY) {
>                 /*
> _at__at_ -3235,7 +3243,7 _at__at_ dlopen_object(const char *name, int fd, Obj_Entry
> *refobj, int lo_flags,
>        (lo_flags & RTLD_LO_EARLY) ? SYMLOOK_EARLY : 0,
>        lockstate) == -1) {
>         objlist_clear(&initlist);
> -       dlopen_cleanup(obj);
> +       dlopen_cleanup(obj, lockstate);
>         if (lockstate == &mlockstate)
>             lock_release(rtld_bind_lock, lockstate);
>         return (NULL);
> _at__at_ -4429,7 +4437,7 _at__at_ trace_loaded_objects(Obj_Entry *obj)
>   * reference count of 0.
>   */
>  static void
> -unload_object(Obj_Entry *root)
> +unload_object(Obj_Entry *root, RtldLockState *lockstate)
>  {
>         Obj_Entry marker, *obj, *next;
>
> _at__at_ -4461,11 +4469,11 _at__at_ unload_object(Obj_Entry *root)
>                         if (next != NULL) {
>                                 init_marker(&marker);
>                                 TAILQ_INSERT_BEFORE(next, &marker, next);
> -                               unload_filtees(obj);
> +                               unload_filtees(obj, lockstate);
>                                 next = TAILQ_NEXT(&marker, next);
>                                 TAILQ_REMOVE(&obj_list, &marker, next);
>                         } else
> -                               unload_filtees(obj);
> +                               unload_filtees(obj, lockstate);
>                 }
>                 release_object(obj);
>         }
>
>
Thanks, kib.  Patch worked perfectly!

- Justin
Received on Thu Mar 09 2017 - 19:36:20 UTC

This archive was generated by hypermail 2.4.0 : Wed May 19 2021 - 11:41:10 UTC