Re: [tomcat-native] branch master updated: Introduce tcn_get_thread_id(void) to reduce code duplication
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 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 +#endif + +#ifdef DARWIN +#include +#endif + +#ifdef __FreeBSD__ +#include +#endif + +#ifdef __linux__ +#include +#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 (
Re: [tomcat-native] branch master updated: Introduce tcn_get_thread_id(void) to reduce code duplication
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 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 +#endif + +#ifdef DARWIN +#include +#endif + +#ifdef __FreeBSD__ +#include +#endif + +#ifdef __linux__ +#include +#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? There are several reasons here. Let me go in detail: * Since we'd like to reduce the dependecy on libapr in the long run, this is a small preparation. * When you take a look at how apr_os_thread_current(void) is implemented you will see that is is mediocre, on Unix(-like) it calls pthread_self(). Our approach for ssl_thread_id(void) is been better than the APR counterpart. I have done a research why pthread_self() isn't enough and an overwhelming amount of voices says that pthread_self() gives very little information, but the thread pointer which is not the thread id. So this is the last resort, but not a real solution. +{ +/* 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. I do not fully understand this. Should the test for z/OS happen before the generic code? The code comes from here [1]. +#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)? That's a very valid point I forgot. I will investigate and test. Thank you for raising! Surprisingly, I did not get a warning about a missing prototype by HP aC. M [1] https://bz.apache.org/bugzilla/show_bug.cgi?id=56210 - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
Re: [tomcat-native] branch master updated: Introduce tcn_get_thread_id(void) to reduce code duplication
-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 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 +#endif + +#ifdef DARWIN > +#include +#endif + +#ifdef __FreeBSD__ +#include > +#endif + +#ifdef __linux__ +#include > +#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? > +{ +/* 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)? - -chris -BEGIN PGP SIGNATURE- Comment: Using GnuPG with Thunderbird - https://www.enigmail.net/ iQIzBAEBCAAdFiEEMmKgYcQvxMe7tcJcHPApP6U8pFgFAl6jEmQACgkQHPApP6U8 pFjvjw/+NcylTnzG4+0N3NulBBv/2dhZPKJJjeH5vkw519CDXUikcy6ObTKZ58XZ 6PFnG6t7KCWAU87IODWV+qPR1sg4sUyhqWi2eC1djN97KpZFkE0lMhyV+gKelMFM wOmRHB6tow1hI6hHkmssX5udGsPWZvKjJYBCiNCaKto7VwQS8tiSy3S8za2RMFnz UJNmi5iirMNRncuIzUhrpK0n49xNWqSLBYjnk9d1jcVKwSydA0oVz3SgKumscnH+ Exakdk/XvDtwTXQLKaIA9S0OvRbj44bCkiuM4Yrc8s0uF3H3tcRAmpv5ADX4IRjn dU0imx8jEZK/SOHP95TeejNcjAocWUPMvQZ0LTTVhn4Ld4qA+TtD8h0IODMdkt4E E+fb1HfYySlRAgASUNEBUKl57t3uRTyQcC2fmjjvWvc4ZqY8QKFVdnFNWIiSylE+ 3xA1ORoNNU35F4RMnqZFSLkkRvulNew8hl/YBNyxPcANhWBe8BTK5amfdGqsdOed KTwpp6o58FzCKs6GSowkWSRClaqqITmtE8kQOo0+ecqKZclW6OLqExTNcS4a9At3 uaYpT2Tt89Ul9WZ4XdE6X1d1hFN4Mg8vsCaO86rTvOlnJaLs9CKsKX4uRuOmgMGY h9qlkKIM1PRPuzGLLkxs+jWnyPRQX2jTO1N5wcUPjqsbEdoH074= =LLLb -END PGP SIGNATURE- - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[tomcat-native] branch master updated: Introduce tcn_get_thread_id(void) to reduce code duplication
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 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_ttcn_load_finfo_class(JNIEnv *, jclass); apr_status_ttcn_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 +#endif + +#ifdef DARWIN +#include +#endif + +#ifdef __FreeBSD__ +#include +#endif + +#ifdef __linux__ +#include +#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) +{ +/* 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; +#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 +} + diff --git a/native/src/ssl.c b/native/src/ssl.c index 57fbb53..98d77eb 100644 --- a/native/src/ssl.c +++ b/native/src/ssl.c @@ -20,14 +20,6 @@ #include "apr_atomic.h" #include "apr_poll.h" -#ifdef __linux__ -#include -#endif - -#ifdef __FreeBSD__ -#include -#endif - #ifdef HAVE_OPENSSL #include "ssl_private.h" @@ -465,30 +457,7 @@ static void ssl_thread_lock(int mode, int type, static unsigned long ssl_thread_id(void) { -/* 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; -#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)(apr_os_thread_current()); -#endif +return (unsigned long)tcn_get_thread_id(); } #if OPENSSL_VERSION_NUMBER < 0x1010L diff --git a/native/src/thread.c b/native/src/thread.c index bddaee3..6696dc0 100644 --- a/native/src/thread.c +++ b/native/src/thread.c @@ -19,5 +19,6 @@ TCN_IMPLEMENT_CALL(jlong, Thread, current)(TCN_STDARGS) { UNREFERENCED_STDARGS; -return (jlong)((unsigned long)apr_os_thread_current()); +return (jlong)tcn_get_thread_id(); } + diff --git a/xdocs/miscellaneous/changelog.xml b/xdocs/miscellaneous/changelog.xml index 0b64cee..a04005a 100644 --- a/xdocs/miscellaneous/changelog.xml +++ b/xdocs/miscellaneous/changelog.