hyc@symas.com writes:
h.b.furuseth@usit.uio.no wrote:
It's called from pool_wrapper(). pool_wrapper() could go active so there can be no pause while that context_reset() call is running.
Oh, ok. This is pretty much a non-issue because threads in the pool stay live until slapd shuts down. The one exception is for a cn=config modify that decreases the size of the pool to below the number of currently active threads, in which case some number of threads will exit immediately. I don't see any potential for conflict here.
Eh? Yes, that's what I was talking about. But I'm not sure what you mean with them exiting immediately - or how they could. Also they stay around until there are no pending requests. A pause request could arrive more or less at any time, I presume. (I don't know if there is a pause while the max threads number is being modified.)
How can a single thread ID be represented by multiple bit patterns?
A struct/union with padding bytes seems the least unlikely possibility.
(...) So in that respect, my preference (not for RE23) is to cast to an integer for the hash, and also to add pthread_<set/get>specific() so thread_keys[] and its hash will only be a fallback solution.
Casting to an int only works with simple types, the point of using the hash was to provide a solution that works for both simple and structured thread IDs.
Yes, except it's not guaranteed to work. Which is why I'm thinking it may be better to fail to compile and provide a fix if/when someone complains. 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?
However note that we do #define ldap_int_thread_equal(a, b) ((a) == (b)) for all our supported thread implementations except pthreads, and for pthreads we can test pthread_<set/get>specific() just fine (as a replacement for thread_keys[]).
OTOH, '==' also works for pointers that are wider than any integer type (and for floating-point thread IDs:-) so I guess I'm arguing against my own suggestion on that one if we add pthread_<set/get>specific() support.
If you want to #ifdef the code so the two cases are handled differently, then just get rid of TID_HASH for the integral case.
No, sould still hash to avoid clustering.
I'm still leary of the pthread TLS support,
Me too. But the old code was broken on real-life systems I have access to, not just on theoretical ones. Still waiting to hear from Kurt if my fix to the first tls.c change fixed it for FreeBSD.
and making that change will require us to update all of the other thread package support, some of which I don't have access to build/test any more (e.g. SunOS4 LWP).
What do you mean? The TLS problem is to extract an unsigned long from a ldap_pvt_thread_t. That needs no support in the thr_*.c files, only to know what kind of type ldap_pvt_thread_t is. Once configure knows what ldap_pvt_thread_t will be typedeffed to it can test if that's a pointer, integer or something worse - and whether it's too wide for a long.
BTW I don't know if the unsigned long needs to be the thread ID or just _some_ unique integer. A question to openssl-users@openssl.org got no answer.
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.
(...) I'm not sure how to determine _which_ multiple of 1MB - start two threads and compare their stack addresses, maybe? In any case, I agree it sounds rather more machine-dependent than the current code.
That's what the LDAP_PVT_THREAD_STACK_SIZE macro tells you. 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.
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.