RFR: 8252324: Signal related code should be shared among POSIX platforms
Coleen Phillimore
coleenp at openjdk.java.net
Mon Oct 5 19:26:47 UTC 2020
On Sat, 3 Oct 2020 17:12:30 GMT, Gerard Ziemski <gziemski at openjdk.org> wrote:
> This is a fresh start for https://github.com/openjdk/jdk/pull/157
>
> Please review this change that refactors common POSIX code into a separate
> file.
>
> Currently there appears to be quite a bit of duplicated code among POSIX
> platforms, which makes it difficult to apply single fix to the signal code.
> With this fix, we will only need to touch single file for common POSIX
> code fixes from now on.
>
> ---------
> ### Progress
> - [x] Change must not contain extraneous whitespace
> - [x] Commit message must refer to an issue
> - [ ] Change must be properly reviewed
>
>
>
> ### Download
> `$ git fetch https://git.openjdk.java.net/jdk pull/497/head:pull/497`
> `$ git checkout pull/497`
Thank you for doing this change! If you can change these places where there are a couple of comments, that would be
great.
Marked as reviewed by coleenp (Reviewer).
src/hotspot/os/posix/signals_posix.hpp line 28:
> 26: #define OS_POSIX_SIGNALS_POSIX_HPP
> 27:
> 28: #include "memory/allocation.hpp"
There's a new memory/allStatic.hpp header file you can include instead.
src/hotspot/os/posix/signals_posix.hpp line 35:
> 33: // Signal number used to suspend/resume a thread
> 34: // do not use any signal number less than SIGSEGV, see 4355769
> 35: static int SR_signum = SIGUSR2;
Can you hide this in the .cpp file so that you can avoid including <signal.h> in the header file?
And use forward declarations for outputStream, ucontext_t, and siginfo_t.
src/hotspot/os/aix/os_aix.cpp line 1459:
> 1457: // that will interfere with OOM killling.
> 1458: PosixSignals::print_signal_handler(st, SIGDANGER, buf, buflen);
> 1459: }
I wonder with some #ifdefs, this could also be moved to posix_signals.cpp. Not for this change but maybe a follow-up
RFE.
-------------
Marked as reviewed by coleenp (Reviewer).
PR: https://git.openjdk.java.net/jdk/pull/497
More information about the hotspot-runtime-dev
mailing list