From owner-freebsd-current@freebsd.org Thu Mar 9 20:36:20 2017 Return-Path: Delivered-To: freebsd-current@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id CEF7FD05513 for ; Thu, 9 Mar 2017 20:36:20 +0000 (UTC) (envelope-from chmeeedalf@gmail.com) Received: from mail-qk0-x22b.google.com (mail-qk0-x22b.google.com [IPv6:2607:f8b0:400d:c09::22b]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (Client CN "smtp.gmail.com", Issuer "Google Internet Authority G2" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 880B7C2 for ; Thu, 9 Mar 2017 20:36:20 +0000 (UTC) (envelope-from chmeeedalf@gmail.com) Received: by mail-qk0-x22b.google.com with SMTP id p64so138915046qke.1 for ; Thu, 09 Mar 2017 12:36:20 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:sender:in-reply-to:references:from:date:message-id :subject:to:cc; bh=KKzfVp0aPwW0jNJnQPt4stxhExwMljO7FBi5/+pMSzo=; b=eD8kqaAjM4Y59h35KKDg7OnkzFyb5qJNaXPFj5J/ULvG/tqKBlo/12EbvLJef0Xaku +ue85Up2RqM/8mxNQB7RnfMDd/ro4X7CXIpFd4WDFbrSEK6+7jYjVz6H5BRDzVrbYxh4 ExZhpQUVzFzKURlTs9BkPYqL2msJITIvMGxB6knUOaE9x3IQpLhPbqAkXfeHFszEjF65 jffqsyU9nljwPrA/RXDtJgFqWacifHvflX7IKsOrjJwSKhwwIUsR57/VsFpH4Ja+y/+r 5H2ZHyVh700GjuY0wk601IFg7JJgOEwtYUTO6H4ypCehrPE02oMBFMiFxZ7nB1hLSyhZ BveA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:sender:in-reply-to:references:from :date:message-id:subject:to:cc; bh=KKzfVp0aPwW0jNJnQPt4stxhExwMljO7FBi5/+pMSzo=; b=Nl14Ts0wI00hOlGxk1DNVOjzwIN0GtAuFarjEteEnZVhaTTZiG82/eAChubw7lXjs1 9Q64wk552eQ8BumEpv2gRjyh+/0Ibe8e4cVaRlL3kvul0vTfeVWxd7/JOimT648uCXQe k/8JaehhkSnF1J7ZPhTGctTZPvsrqEMH0h84s0Sae5EIcy2B9irIJ+z+z89UUExDxWFa pouMtaztOW6KeEdcU0e0lTQNTewyIwKZGQWLtGEGbs+llrwVoWZgA16zFCSnAg9GbriE ovfNYr4gvsISpJqBakmD4aC4HAnAHrCUBH/KOarn7L6kNjTGBJiXO1N/6nNhtV6wWZ5G wnNw== X-Gm-Message-State: AMke39lVMCBXWIt+xTZt/UYmPkSPAOjFnd3oNWgV4sKa90R+xTa7RhWK88jc0kUyYCUwmv/QbLP5QNH+jI6sjw== X-Received: by 10.200.62.137 with SMTP id y9mr16067553qtf.182.1489091779620; Thu, 09 Mar 2017 12:36:19 -0800 (PST) MIME-Version: 1.0 Sender: chmeeedalf@gmail.com Received: by 10.12.172.240 with HTTP; Thu, 9 Mar 2017 12:36:19 -0800 (PST) In-Reply-To: <20170309163052.GC16105@kib.kiev.ua> References: <20170309163052.GC16105@kib.kiev.ua> From: Justin Hibbits Date: Thu, 9 Mar 2017 14:36:19 -0600 X-Google-Sender-Auth: A5anGlqxG0D6n0JO0NBGrlVs-vc Message-ID: Subject: Re: hang in dlclose() on rtld lock (powerpc64) To: Konstantin Belousov Cc: FreeBSD Current Content-Type: text/plain; charset=UTF-8 X-Content-Filtered-By: Mailman/MimeDel 2.1.23 X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: Discussions about the use of FreeBSD-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 09 Mar 2017 20:36:20 -0000 On Thursday, March 9, 2017, Konstantin Belousov 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=, > > tsp=) > > at /home/chmeee/freebsd/pristine/lib/libthr/thread/thr_umtx.c:325 > > #3 0x00000000505965f0 in _thr_rwlock_wrlock (tsp=, > > rwlock=) > > 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 , > > 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=) > > 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 to continue, or q 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 > @@ -77,6 +77,7 @@ 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); > @@ -98,7 +99,7 @@ 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); > @@ -142,7 +143,7 @@ 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 *, > @@ -2104,13 +2105,13 @@ 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; > } > } > @@ -2121,14 +2122,14 @@ 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 > @@ -3015,15 +3016,23 @@ 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); > > @@ -3035,19 +3044,18 @@ 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; > } > > @@ -3123,13 +3131,13 @@ 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 * > @@ -3178,7 +3186,7 @@ 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) { > /* > @@ -3235,7 +3243,7 @@ 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); > @@ -4429,7 +4437,7 @@ 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; > > @@ -4461,11 +4469,11 @@ 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