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

[h.b.furuseth@usit.uio.no]

I wonder if we've been waiting for each other...  Anyway, I've committed
some pending fixes now that HEAD passes all tests for me.  I've some
more cleanup, but if we are close to a freeze I don't know if that means
I should rush it in or wait until afterwards.


>> We did have code which compared thread IDs with '==' for
>> quite some time.  Do you remember if anyone complained, or if the
>> switch to using ldap_pvt_thread_equal() was a "just in case" fix?
>
> Yes, I needed to use pthread_equal() on OS/390 since their thread type
> was a structure.

I'm not sure, but I gather that's derived from the DCE threads reference
implementation where pthread_t has three members 'field1', 'field2' and
'field3'.  tls.c can't stuff all of them in a long, so the old one may
not get a unique ID.  OTOH DCE has a pthread_getunique_np() call which
uses just the 2nd field as a unique integer thread ID, I expect we could
use that.  Except I don't have a host to test such a change on.  Dunno
why pthread_equal checks all three fields.  I've asked on
comp.programming.threads.

Also I note that the DCE struct will get four padding bytes on a 64-bit
host which aligns the struct at 8*n addresses, and the compiler is not
obliged to preserve these during struct copy.

On the other hand DCE threads are based on an extremely outdated Posix
draft, so I don't know how relevant it is now.

Another time when tls.c code loses information is if pointers are wider
than long, which does happen.  (Don't know how it is nowadays, but I
remember plenty of postings about it some years ago.)


[Using address of the thread's stack]
>>> That's what the LDAP_PVT_THREAD_STACK_SIZE macro tells you.

As modified by PTHREAD_STACK_MIN and pthread_attr_getguardsize().  And
slight other variations, apparently.  I tried on a few hosts and Solaris
would a few times use some extra space near the stack of a thread or two.

OTOH we could allocate stack memory ourselves and then use
pthread_attr_setstack() to put the stack just where we want it.

However this doesn't help tls.c with threads that have been created by
_other_ packages.  And the SSL id_callback function apparently has no
way to return failure:-(

All in all I still like a thread-specific key better - they are the
standard way of doing things like this.  We only need it for pthreads
since the other thread types as far as I can tell have integer IDs.

>>> It's really
>>> not hard.  If we were to disallow increasing the number of threads at
>>> runtime, we could simply allocate a block of thread stacks all at once,
>>> which would then allow our thread IDs to be simple integers from 0-N.

IIRC disallowing that would fix some other tpool problem too, but I
don't remember what.

>> Hmm.  I asked the wrong question, should have read some man pages first.
>> Still, thr_cthreads.c & thr_lwp.c don't use LDAP_PVT_THREAD_STACK_SIZE.
>> man pthread_attr_getstacksize says it sets the _minimum_ stack size, but
>> perhaps several M is large enough that one can expect it won't get
>> multiples of that.
>
> That kinda implies that no one has been using the cthreads or lwp support
> recently, since I doubt their default stack size would work with back-bdb's
> filterindex demands.

Which led me to notice that the setstacksize call now _decreases_ the
stack size on some hosts.  I don't suppose that was intentional, so I
guess it should be
    size_t cursize;
    if( pthread_attr_getstacksize( &attr, &cursize )
        || cursize < LDAP_PVT_THREAD_STACK_SIZE )
        pthread_attr_setstacksize( &attr, LDAP_PVT_THREAD_STACK_SIZE );
in thr_posix and I don't know what in other thr_* files.  A RE24 change
I guess, since reduces the max number of threads before slapd gets into
trouble.

-- 
Regards,
Hallvard