Re: [RFC][BZ #19329] high level description of pthread_create vs dlopen races

[Szabolcs Nagy <szabolcs dot nagy at arm dot 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).
>