tomcat-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Rainer Jung <rainer.j...@kippdata.de>
Subject Re: Bug that spans tomcat and tomcat-native
Date Tue, 28 Jun 2016 15:51:09 GMT
Am 28.06.2016 um 16:07 schrieb Mark Thomas:
> On 28/06/2016 12:28, Mark Thomas wrote:
>> On 28/06/2016 11:34, Rainer Jung wrote:
>>> Am 28.06.2016 um 11:15 schrieb Mark Thomas:
>>
>> <snip />
>>
>>>> Index: src/ssl.c
>>>> ===================================================================
>>>> --- src/ssl.c    (revision 1750259)
>>>> +++ src/ssl.c    (working copy)
>>>> @@ -420,6 +420,10 @@
>>>>      return psaptr->PSATOLD;
>>>>  #elif defined(WIN32)
>>>>      return (unsigned long)GetCurrentThreadId();
>>>> +#elif defined(DARWIN)
>>>> +    uint64_t tid;
>>>> +    pthread_threadid_np(NULL, &tid);
>>>> +    return (unsigned long)tid;
>>>>  #else
>>>>      return (unsigned long)(apr_os_thread_current());
>>>>  #endif
>>>>
>>>>
>>>> I want to do some similar testing for Linux before adding what I suspect
>>>> will be a very similar block using gettid().
>>>
>>> We could either add something to configure.in. Untested:
>>>
>>> Index: native/configure.in
>>> ===================================================================
>>> --- native/configure.in (revision 1750462)
>>> +++ native/configure.in (working copy)
>>> @@ -218,6 +218,9 @@
>>>      *-solaris2*)
>>>          APR_ADDTO(TCNATIVE_LIBS, -lkstat)
>>>          ;;
>>> +    *linux*)
>>> +        APR_ADDTO(CFLAGS, -DTCNATIVE_LINUX)
>>> +        ;;
>>>      *)
>>>          ;;
>>>  esac
>>>
>>>
>>> and then use a
>>>
>>> #ifdef TCNATIVE_LINUX
>>>
>>> or we copy some other more direct linux check from e.g. APR:
>>>
>>> #ifdef __linux__
>>>
>>> The latter looks simpler, but the version above is based on all the
>>> logic put into config.guess.
>>
>> I'd go with the __linux__ option as that is consistent with what we
>> already use in os/unix/system.c
>>
>> I'm not against the change to configure.in, I just think we should be
>> consistent with how we do this throughout the code base.
>
> I've confirmed that the same problem occurs with hash bucket selection
> on linux and that switching to gettid() fixes that problem.
>
> I'm going to go ahead with the 1.2.8 release shortly. We can continue to
> refine this as necessary and have a more complete fix in 1.2.9.

I did a quick check on Solaris. apr_os_thread_current() uses 
pthread_self on Solaris like on Linux (actually on any Unix type OS), 
but unlike Linux where this returns a address which is either 32 or 64 
bit aligned depending on address size, on Solaris you get an increasing 
number starting with 1 for the first thread and incremented by one for 
each following thread. Thread IDs do not get reused in the same process, 
even if the thread finished, but thread IDs are common between different 
processes, because they always start with 1. So Solaris should be fine 
as-is.

>>>> I do think OS specific code to get the real thread ID is the way to go
>>>> here until we can switch to OpenSSL 1.1.0 at which point the problem
>>>> should go away.
>>>
>>> Agreed, but cleaning up at the end of a thread would be good as well,
>>> e.g. if the executor is configured to renew threads when a context is
>>> reloaded we can accumulate lots of old hash entries.
>>
>> Agreed.
>>
>> Mark

Rainer

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Mime
View raw message