Am 2020-04-24 um 18:23 schrieb Christopher Schultz:
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

Michael,

On 4/23/20 18:42, micha...@apache.org wrote:
This is an automated email from the ASF dual-hosted git
repository.

michaelo pushed a commit to branch master in repository
https://gitbox.apache.org/repos/asf/tomcat-native.git


The following commit(s) were added to refs/heads/master by this
push: new f95f531  Introduce tcn_get_thread_id(void) to reduce code
duplication f95f531 is described below

commit f95f531e98278cc7555367084b967e3550734559 Author: Michael
Osipov <micha...@apache.org> AuthorDate: Thu Apr 23 18:52:44 2020
+0200

Introduce tcn_get_thread_id(void) to reduce code duplication

At two spots (ssl.c and thread.c) we need to obtain the native
thread id. This has been done with two different approaches. Move
out to tcn_get_thread(void) which uses the previous
ssl_thread_id(void) implementation while the previous functions
delegate to the new one. apr_os_thread_current(void) is not used
anymore which does internally the same thing as
ssl_thread_id(void) was doing.

Also add properly #ifdefs for Windows and macOS for function
prototype includes. --- native/include/tcn.h              |  1 +
native/src/jnilib.c               | 45
+++++++++++++++++++++++++++++++++++++++ native/src/ssl.c
| 33 +--------------------------- native/src/thread.c
|  3 ++- xdocs/miscellaneous/changelog.xml |  5 ++++- 5 files
changed, 53 insertions(+), 34 deletions(-)

diff --git a/native/include/tcn.h b/native/include/tcn.h index
2b2ae59..d2f316b 100644 --- a/native/include/tcn.h +++
b/native/include/tcn.h @@ -175,6 +175,7 @@ char
*tcn_strdup(JNIEnv *, jstring); char           *tcn_pstrdup(JNIEnv
*, jstring, apr_pool_t *); apr_status_t
tcn_load_finfo_class(JNIEnv *, jclass); apr_status_t
tcn_load_ainfo_class(JNIEnv *, jclass); +unsigned long
tcn_get_thread_id(void);

#define J2S(V)  c##V #define J2L(V)  p##V diff --git
a/native/src/jnilib.c b/native/src/jnilib.c index dae3ade..e88d4d5
100644 --- a/native/src/jnilib.c +++ b/native/src/jnilib.c @@ -23,6
+23,22 @@

#include "tcn_version.h"

+#ifdef WIN32 +#include <Windows.h> +#endif + +#ifdef DARWIN
+#include <pthread.h> +#endif + +#ifdef __FreeBSD__ +#include
<pthread_np.h> +#endif + +#ifdef __linux__ +#include
<sys/syscall.h> +#endif + #ifdef TCN_DO_STATISTICS extern void
sp_poll_dump_statistics(); extern void
sp_network_dump_statistics(); @@ -481,3 +497,32 @@ jint
tcn_get_java_env(JNIEnv **env) } return JNI_OK; } + +unsigned long
tcn_get_thread_id(void)

Why not simple call apr_os_thread_current() instead of writing a new
function? Or is the intention to get away from using APR?

Here is the difference between pthread_self() and OS-specific calls:

FreeBSD:
osipovmi@deblndw011x:~/var/Projekte/tomcat-native (master *%=)
$ uname -a
FreeBSD deblndw011x.ad001.siemens.net 12.1-STABLE
osipovmi@deblndw011x:~/var/Projekte/tomcat-native (master *%=)
$ # pthread_self()
osipovmi@deblndw011x:~/var/Projekte/tomcat-native (master *%=)
$ java -cp dist/classes/java:. -Djava.library.path=native/.libs TcnativeTest
Thread Id: 34381534464
osipovmi@deblndw011x:~/var/Projekte/tomcat-native (master *%=)
$ java -cp dist/classes/java:. -Djava.library.path=native/.libs TcnativeTest
Thread Id: 34381534464
osipovmi@deblndw011x:~/var/Projekte/tomcat-native (master *%=)
$ java -cp dist/classes/java:. -Djava.library.path=native/.libs TcnativeTest
Thread Id: 34381534464
$ # pthread_getthreadid_np() on FreeBSD
osipovmi@deblndw011x:~/var/Projekte/tomcat-native (master %=)
$ make -C native/
osipovmi@deblndw011x:~/var/Projekte/tomcat-native (master %=)
$ java -cp dist/classes/java:. -Djava.library.path=native/.libs TcnativeTest
Thread Id: 100654
osipovmi@deblndw011x:~/var/Projekte/tomcat-native (master %=)
$ java -cp dist/classes/java:. -Djava.library.path=native/.libs TcnativeTest
Thread Id: 100839

HP-UX:
osipovmi@deblndw024v:~/tomcat-native (master *%=)
$ uname -a
HP-UX deblndw0 B.11.31 U ia64 HP-UX
osipovmi@deblndw024v:~/tomcat-native (master *%=)
$ # pthread_self() only
osipovmi@deblndw024v:~/tomcat-native (master *%=)
$ java -cp dist/classes/java:. -Djava.library.path=native/.libs TcnativeTest
Thread Id: 1
osipovmi@deblndw024v:~/tomcat-native (master *%=)
$ java -cp dist/classes/java:. -Djava.library.path=native/.libs TcnativeTest
Thread Id: 1

GNU/Linux:
osipovmi@deblndw012x: ~/tomcat-native (master *%=)
$ uname -a
Linux deblndw012x.ad001.siemens.net 3.10.0-1127.el7.x86_64 #1 SMP Tue Feb 18 
16:39:12 EST 2020 x86_64 x86_64 x86_64 GNU/Linux
$ # pthread_self()
osipovmi@deblndw012x: ~/tomcat-native (master *%=)
$ java -cp dist/classes/java:. -Djava.library.path=native/.libs TcnativeTest    
Thread Id: 140074735822592
osipovmi@deblndw012x: ~/tomcat-native (master *%=)
$ java -cp dist/classes/java:. -Djava.library.path=native/.libs TcnativeTest
Thread Id: 139635928618752
osipovmi@deblndw012x: ~/tomcat-native (master *%=)
$ java -cp dist/classes/java:. -Djava.library.path=native/.libs TcnativeTest
Thread Id: 139627128493824
osipovmi@deblndw012x: ~/tomcat-native (master *%=)
$ make -C native/
osipovmi@deblndw012x: ~/tomcat-native (master *%=)
$ # syscall()
osipovmi@deblndw012x: ~/tomcat-native (master *%=)
$ java -cp dist/classes/java:. -Djava.library.path=native/.libs TcnativeTest
Thread Id: 17672
osipovmi@deblndw012x: ~/tomcat-native (master *%=)
$ java -cp dist/classes/java:. -Djava.library.path=native/.libs TcnativeTest
Thread Id: 17730
osipovmi@deblndw012x: ~/tomcat-native (master *%=)
$ java -cp dist/classes/java:. -Djava.library.path=native/.libs TcnativeTest
Thread Id: 17784

There is a difference.

+{ +    /* OpenSSL needs this to return an unsigned long.  On
OS/390, the pthread +     * id is a structure twice that big.  Use
the TCB pointer instead as a +     * unique unsigned long. +
*/ +#ifdef __MVS__ +    struct PSA { +        char unmapped[540]; +
unsigned long PSATOLD; +    } *psaptr = 0; + +    return
psaptr->PSATOLD;

I think we might want to put the above #ifdef as the LAST one in the
list. I think if we can call pthread_self, we should, even if this
other technique will work.

+#elif defined(WIN32) +    return (unsigned
long)GetCurrentThreadId(); +#elif defined(DARWIN) +    uint64_t
tid; +    pthread_threadid_np(NULL, &tid); +    return (unsigned
long)tid; +#elif defined(__FreeBSD__) +    return (unsigned
long)pthread_getthreadid_np(); +#elif defined(__linux__) +
return (unsigned long)syscall(SYS_gettid); +#else +    return
(unsigned long)pthread_self(); +#endif

Can we guarantee that pthread_self() will be available "by default"
since it's predicate-less, here? I wasn't able to quickly find a
reference for how to check for pthreads. Maybe #ifdef(_PTHREAD_H)?

Fixed in 19124fd730c8ca4d080d1ebeed4272944629373e. pthread.h was previously included as a transitive CPP dependency. It is now explicit.

The ifdef will not work because pthread.h across OSes does not contain this define. See:

FreeBSD:
#ifndef _PTHREAD_H_
#define _PTHREAD_H_

GNU/Linux:
#ifndef _PTHREAD_H
#define _PTHREAD_H  1

macOS:

#ifndef _PTHREAD_H
#define _PTHREAD_H

HP-UX:
#ifndef _SYS_PTHREAD_INCLUDED
#define _SYS_PTHREAD_INCLUDED

If you know a portable way, please tell.

Michael

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

Reply via email to