RFR (M): JDK-8200298 Unify all unix versions of libjsig/jsig.c

Magnus Ihse Bursie magnus.ihse.bursie at oracle.com
Wed Mar 28 10:15:03 UTC 2018

On 2018-03-27 18:24, Thomas Stüfe wrote:
> Hi Magnus,
> just a cursory look, will look in greater detail tomorrow.
> This is good, thanks for doing this.
> As I wrote offlist, please remove the painfully wrong AIX 
> "workarounds". I do not know why we did this - the original code is 
> quite old - my assumption is that dlsym(RTLD_NEXT) was not working as 
> expected on older AIX versions. Well, it should work now according to 
> IBMs manpages. Will test later.
> __thread is not Posix. I would prefer pthread_get/set_specific 
> instead, which is more portable.

I have surrounded this code with #ifdef MACOSX now.

Here is an updated webrev. It includes the changes requested by you and 

* No more AIX workarounds
* Solaris use pthreads
* The "reentry" code is #ifdef MACOSX.

I also assumed that NSIG is available and working on Solaris. I'll let 
David decide if he is happy with that. The alternative is to go back to 
the Solaris-specific code that allocates sact on the heap.

Webrev: http://cr.openjdk.java.net/~ihse/JDK-8200298-unify-libjsig/webrev.05

Once again, here is also a webrev that shows the difference between the 
original files and the new, unified file. Even if it's hard to read, it 
shows what the effects will be for each platform.



> Thanks!
> Thomas
> On Tue, Mar 27, 2018 at 11:42 AM, Magnus Ihse Bursie 
> <magnus.ihse.bursie at oracle.com <mailto:magnus.ihse.bursie at oracle.com>> 
> wrote:
>     When I was about to update jsig.c, I noticed that the four copies
>     for aix, linux, macosx and solaris were basically the same, with
>     only small differences. Some of the differences were not even well
>     motivated, but were likely the result of this code duplication
>     causing the code to diverge.
>     A better solution is to unify them all into a single unix version,
>     with the few platform-specific changes handled on a per-platform
>     basis.
>     I've made the following notable changes:
>     * I have removed the version check for Solaris. All other
>     platforms seem to do fine without it, and in general, we don't
>     mistrust other JDK libraries. An alternative is to add this
>     version check to all other platforms instead. If you think this is
>     the correct course of action, let me know and I'll fix it.
>     * Solaris used to have a dynamically allocated sact, instead of a
>     statically allocated array as all other platforms have. It's not
>     likely to be large, and the size is known at compile time, so
>     there seems to be no good reason for this.
>     * Linux and macosx used a uint32_t/uint64_t instead of sigset_t
>     for jvmsigs, as aix and solaris do. This is a less robust
>     solution, and the added checks show that it has failed in the
>     past. Now all platforms use sigset_t/sigismember().
>     Also worth noting:
>     * Solaris is not using pthreads, but it's own thread library,
>     which accounts for most of the #ifdef SOLARIS.
>     * In general, if an implementation was needed on one platform, but
>     has no effect or is harmless on others, I've kept it on all
>     platforms instead of sprinkling the code with #ifdefs.
>     To facilitate code review, here is a specially crafted webrev that
>     shows the differences compared to each of the individual, original
>     per-OS versions of jsig.c:
>     http://cr.openjdk.java.net/~ihse/JDK-8200298-unify-libjsig/webrev.01
>     <http://cr.openjdk.java.net/%7Eihse/JDK-8200298-unify-libjsig/webrev.01>
>     Bug: https://bugs.openjdk.java.net/browse/JDK-8200298
>     <https://bugs.openjdk.java.net/browse/JDK-8200298>
>     WebRev:
>     http://cr.openjdk.java.net/~ihse/JDK-8200298-unify-libjsig/webrev.03
>     <http://cr.openjdk.java.net/%7Eihse/JDK-8200298-unify-libjsig/webrev.03>
>     /Magnus

More information about the hotspot-runtime-dev mailing list