Re: (ITS#4943) tpool.c pause vs. finish

[hyc@symas.com]

Hallvard B Furuseth wrote:
> hyc@symas.com writes:
>> h.b.furuseth@usit.uio.no wrote:
>>> * When a thread is finishing, make it go active for context_reset().
>>>   Should make slapi module destructors safer, and I don't see that it
>>>   can hurt.
>> I don't understand this.
> 
> Let me dig a bit back in the ITS...
>   "context_reset() can call connection_fake_destroy(), which via
>   slapi_int_free_object_extensions() calls slapi module destructors,
>   which can do who-knows-what."

context_reset() can only be called from the main thread. It can (must) never be 
called by anything running from the pool.

>>> * Scheduling: If several threads call pool_pause(), then once there is a
>>>   pause tpool does not schedule them all.  They could get handled then,
>>>   or another thread could undo the pause so tpool would wait to pause
>>>   again.  Is that deliberate?
>> I don't understand the question. The point of the pause is to prevent
>> any other thread (in the pool) from running. Why should tpool schedule
>> any other threads at this point?
> 
> Two threads call pool_pause().  Eventually the pool gets paused, and one
> pool_pause() call returns.  When that thread calls pool_resume(), the
> other thread waiting in pool_pause() may or may not get scheduled.

OK, that sounds like a bug.

>>>   - pool_context() breaks if several ldap_pvt_thread_t bit patterns can
>>>     represent the same thread ID: TID_HASH() would give different hashes
>>>     for the same thread, and pool_context() stops searching when it hits
>>>     a slot with ctx == NULL.  (My previous bug report was a bit wrong.)
>> How can a single thread ID be represented by multiple bit patterns?
> 
> A struct/union with padding bytes seems the least unlikely possibility.

Any implementation that leaves uninitialized padding bytes would be a bug. 
Tools like valgrind would barf all over them.

> A pointer in hardware where several address representations map to the
> same physical address, and the compiler handles that by normalizing
> pointers when they are compared/subtracted.  Like DOS "huge" pointers
> would have been if the compiler normalized them when comparing them
> instead of when incrementing/decrementing them.  20-bit address bus,
> 32-bit pointers, physical address = 16 * <segment half of pointer> +
> <offset half of pointer>.

Nobody voluntarily uses such memory models today. The very mention of "DOS" 
invalidates this discussion since there is no threading environment there.

>>>     The best fix would be to use use real thread-specific data instead.
>>>     Just one key with the ctx for now, that minimizes the changes.  OTOH
>>>     it also means we'll do thread-specific key lookup twice - first in
>>>     pthread to get the ctx, and then in ltu_key[] to find our data.
>>>     Anyway, I said I'd do that later but might as well get on with it,
>>>     at least for pthreads.  Except I don't know if that's OS-dependent
>>>     enough that it should wait for RE24?
>> The best fix may be to just use the address of the thread's stack as the
>> thread ID. That's actually perfect for our purpose.
> 
> How does a function called from somewhere inside the thread know that
> address?  That's almost what the user context is, and thus what
> thread_keys[] maps the thread ID to.

It requires some degree of machine-dependence, which is why I never bothered to 
code it. The principle is simple though - all of our thread stacks are some 
multiple of 1MB in size. Take the address of any local variable, mask off the 
low 20 bits, and you have a unique thread ID (+/- some additional 
masking/shifting based on the actual thread stack size).

-- 
   -- Howard Chu
   Chief Architect, Symas Corp.  http://www.symas.com
   Director, Highland Sun        http://highlandsun.com/hyc/
   Chief Architect, OpenLDAP     http://www.openldap.org/project/