RFR (M): JDK-8200298 Unify all unix versions of libjsig/jsig.c
Magnus Ihse Bursie
magnus.ihse.bursie at oracle.com
Thu Apr 5 08:07:23 UTC 2018
On 2018-04-04 09:59, David Holmes wrote:
> Hi Magnus,
> Sorry for the delay ...
> On 28/03/2018 8:15 PM, Magnus Ihse Bursie wrote:
>> 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 David:
>> * No more AIX workarounds
>> * Solaris use pthreads
>> * The "reentry" code is #ifdef MACOSX.
> That all seems good.
>> 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.
> Unfortunately NSIG is a problem on Solaris:
> It's use also precludes the use of the real-time signals - which
> limits Linux as well. But I'm not completely clear on exactly how
> signals may be numbered in their entirety so I wouldn't necessarily
> suggest trying to use SIGRTMAX+1 as the number of available signals,
> other than on Solaris.
Ok. I hope I understand you correctly. I have replaced NSIG with
MAX_SIGNALS, which is defined as NSIG on all platforms except Solaris,
where it is defined as SIGRTMAX+1.
(8th time's a charm..?)
>> 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.
>>> 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
>>> 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:
>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8200298
More information about the hotspot-runtime-dev