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

Thomas Stuefe stuefe at openjdk.java.net
Tue Oct 6 06:06:41 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`

Looks good to me. All remarks are just proposals for follow ups. Our nightly tests on AIX went through with the current
version of the patch. Ship it. Thanks for your work!

src/hotspot/os/bsd/os_bsd.cpp line 2285:

> 2283:
> 2284: void os::SuspendedThreadTask::internal_do_task() {
> 2285:   if (PosixSignals::do_suspend(_thread->osthread())) {

Future RFE: os::SuspendedThreadTask::internal_do_task() can be unified across posix platforms.

src/hotspot/os/bsd/os_bsd.cpp line 2152:

> 2150:   if (!ReduceSignalUsage) {
> 2151:     PosixSignals::jdk_misc_signal_init();
> 2152:   }

This whole section could be unified too into an own PosixSignals::initial_signal_stuff() function. Potentially for a
future RFE.

src/hotspot/os/posix/signals_posix.cpp line 104:

> 102: static const struct {
> 103:   int sig; const char* name;
> 104: }

Please remove newlines. This is one declaration:

static const struct {
 ,,
} g_signal_info[] = {

src/hotspot/os/posix/signals_posix.cpp line 454:

> 452:                                                void* ucontext,
> 453:                                                int abort_if_unrecognized);
> 454: #endif

Future cleanup: I think it will be difficult to unify the platform specific signal handlers (but worthwhile). But the
naming could be at least the same, no reason to have the platform in the name.

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

Marked as reviewed by stuefe (Reviewer).

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


More information about the hotspot-runtime-dev mailing list