RFR: 8252324: Signal related code should be shared among POSIX platforms [v4]

[Gerard Ziemski]

On Mon, 28 Sep 2020 20:12:36 GMT, Gerard Ziemski <gziemski at openjdk.org> wrote:

>> Hi Gerard,
>> Thank you for tackling this long overdue cleanup and conciliation of the the signal management code! Great work on a
>> tedious task.
>> Overall this looks okay but I confess it is very hard to compare each previous platform version with the new shared
>> version. I'm glad the AIX folk have take an indepth look there.
>> I have a few minor comments in specific files below.
>> 
>> I would also suggest naming the file posixSignal.cpp/hpp as that is the more common naming pattern.
>> 
>> Future cleanup: do we really need JVM_handle_<os>_signal to be cpu specific? I can imagine one copy of this with a few
>> ifdefs or newly introduced os::pd_* functions.
>> Thanks,
>> David
>
>> Hi Gerard,
>> Thank you for tackling this long overdue cleanup and conciliation of the the signal management code! Great work on a
>> tedious task.
>> Overall this looks okay but I confess it is very hard to compare each previous platform version with the new shared
>> version.
> 
> Sorry about that, I do understand how hard it was to review it and really appreciate it!
> 
> 
>> I'm glad the AIX folk have take an indepth look there.
> 
> Me too!
> 
>> I have a few minor comments in specific files below.
>> 
>> I would also suggest naming the file posixSignal.cpp/hpp as that is the more common naming pattern.
> 
> I based the name on the other files in that directory, i.e.:
> 
> jvm_posix.cpp
> os_posix.cpp
> semaphore_posix.cpp
> threadLocalStorage_posix.cpp
> vmError_posix.cpp
> 
> so I introduced:
> 
> signals_posix.cpp
> 
>> Future cleanup: do we really need JVM_handle_<os>_signal to be cpu specific? I can imagine one copy of this with a few
>> ifdefs or newly introduced os::pd_* functions.
>> Thanks,
>> David
> 
> Thank you David very much for catching the merge issues and review!

I will merge again with current, review the changes again looking for weird merge issue, push here, then re-test...

-------------

PR: https://git.openjdk.java.net/jdk/pull/157