public inbox for libc-alpha@sourceware.org
 help / color / mirror / Atom feed
From: Szabolcs Nagy <szabolcs.nagy@arm.com>
To: Torvald Riegel <triegel@redhat.com>
Cc: <nd@arm.com>, GNU C Library <libc-alpha@sourceware.org>
Subject: Re: [RFC][BZ #19329] high level description of pthread_create vs dlopen races
Date: Thu, 05 Jan 2017 17:53:00 -0000	[thread overview]
Message-ID: <586E8803.3070000@arm.com> (raw)
In-Reply-To: <585A9F58.8040603@arm.com>

On 21/12/16 15:27, Szabolcs Nagy wrote:
> i tried to collect the requirements, the current implementation
> is far from it: as-safety of tls access and ctor/dtor without
> locks are difficult to fix. if as-safe tls access is not required
> then the concurrency fixes are much simpler.

following steps might work for incremental bug fixing:

1) fix the gen counter to be 64bit on 32bit targets
  and remove all overflow checks and related aborts.
  (requires 64bit atomic load and store.)

2) make sure slotinfo update does not access link_map
  of a module that may be dlclosed concurrently.
  (the tricky bit is to handle static tls initialization
  in pthread_create: it needs either sync with dlclose
  or remembering static tls info in the global slotinfo
  list, i'd try to do the later).

3) fix pthread_create so it initializes dtv with the
  right size for a particular generation and iterates
  over slotinfo entries of modules with static tls
  correctly to initialize tls and related dtv entries.

this would fix BZ 19329 and the following issues would remain:

- tls access allocates and frees dtv entries and may
  resize dtv and may lock the load_lock.

- dlopen and pthread_create aborts if resize_dtv fails.
  and dlopen may leave inconsistent state behind if
  other allocation fails.

- dtors and ctors are run while the load_lock is held.

these seem hard to fix to me.

> simplified model:
> 
> m: module.
> m.i: modid.
> t: thread.
> t.dtv: dtv pointer.
> t.dtv[i]: dtv entry i (tls for t,m where m.i==i).
> 
> constraints:
> 
> tls of a currently loaded module m in thread t is at t.dtv[m.i].
> 
> m.i is unique during the lifetime of m: it is assigned before
> ctors of m are entered and may be reused after dtors of m return.
> 
> tls access to m in thread t is only valid during the lifetime
> of t and m (after ctors of m start and before dtors of m end).
> 
> during the lifetime of a thread its dtv may be updated:
> t.dtv may need to be resized (if an m is loaded with larger m.i).
> t.dtv[i] may need to be freed (if an m is dlclosed).
> t.dtv[i] may need to be allocated (if an m is dlopened).
> 
> if dtv updates are not immediate for all threads at dlopen and
> dlclose time, then the threads need a way to tell if t.dtv[i]
> is out-of-date in case modid i is reused by the time the dtv
> update happens. (this can be done by counting dlopen/dlclose
> calls and remembering the counter of the last dtv update and
> globally tracking the last counter for each modid i, if
> global_counter[i] > t.dtv_counter then t.dtv[i] is out-of-date.
> such counter should be 64bit).
> 
> dtv update consists of three kind of operations:
> 1) allocate dtv and dtv entries (malloc)
> 2) unset out-of-date entries (free)
> 3) resize dtv, set dtv entries (memcpy and ptr update)
> 
> 1) alloc:
> pthread_create and dlopen needs to be synchronized such that
> either sync in dlopen or sync in pthread_create happens before
> the other (for all dlopen/pthrea_create pairs), the one that
> happens later should do the allocation.
> (this is needed because right after dlopen and pthread_create
> tls access is valid, but must be as-safe so it cannot easily
> allocate).
> 
> 2) free:
> t.dtv[m.i] should be freed eventually after dlclose of m or
> after t exits. this is difficult because t.dtv[m.i] need to
> be updated if m.i is reused and the tls of the new module is
> accessed, but tls access cannot do the free (not as-safe).
> so the options are
> - dlclose of m frees t.dtv[m.i] for all t (non-trivial).
> - allocated dtv entry pointers are remembered somewhere
>   and garbage collected in some way (such that overwriting
>   t.dtv[i] does not leak memory).
> 
> 3) update dtv pointer and dtv entry:
> either dlopen stops all threads and takes care of dtv updates
> or it has to be done during tls access lazily in which case
> signals may need to be blocked.
> 
> an m with static tls is a special case:
> 1) if m is already loaded when t is created, then pthread_create
> needs to do setups (copy tls init image) that requires accessing
> m, so either pthread_create needs to sync with dlclose or it
> is invalid to unload an m with static tls, in the later case
> pthread_create should be able to walk the currently loaded
> modules and tell if they have static tls without accessing the
> module structures that are freed during dlclose.
> 2) if m is loaded after t is created, then dlopen should do the
> setup for the current thread, but i think it has to do the setup
> for other threads as well (?). (in principle static tls cannot
> be dynamically loaded/unloaded but i'm not sure what are the
> requirements if glibc internal libraries are dlopened.)
> 
> indirectly related to tls:
> 
> ctor,dtor handling should be callback safe: dlopen and dlclose
> must not hold internal locks otherwise correct ctor/dtor code may
> deadlock.
> 
> dlopen and pthread_create should roll back state changes on allocation
> failure and report the errors (some state changes may be harmless
> this needs further analysis).
> 

      reply	other threads:[~2017-01-05 17:53 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-11-28 19:03 Szabolcs Nagy
2016-12-05 19:24 ` Torvald Riegel
2016-12-06 17:23   ` Szabolcs Nagy
2016-12-06 17:48     ` Joseph Myers
2016-12-06 18:31       ` Szabolcs Nagy
2016-12-07 14:08     ` Torvald Riegel
2016-12-21 15:27       ` Szabolcs Nagy
2017-01-05 17:53         ` Szabolcs Nagy [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=586E8803.3070000@arm.com \
    --to=szabolcs.nagy@arm.com \
    --cc=libc-alpha@sourceware.org \
    --cc=nd@arm.com \
    --cc=triegel@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).