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