On Thu, 27 May 2021 22:05:55 GMT, Leonid Mesnik <lmes...@openjdk.org> wrote:
> EFH is improved to process cores and get mixed stack traces with jhsdb and
> native stack traces with gdb/lldb. It might be useful because hs_err doesn't
> contain info about all threads, sometimes it is even not generated.
@lmesnik , how has this been tested?
test/failure_handler/Makefile line 41:
> 39: CONF_DIR = src/share/conf
> 40:
> 41: JAVA_RELEASE = 7
could you please update `DEPENDENCES` section in `test/failure_handler/README`?
test/failure_handler/src/share/classes/jdk/test/failurehandler/ToolKit.java
line 69:
> 67: }
> 68: } catch (IOException ioe) {
> 69: // just silently skip gzipped core processing
we don't silently ignore exceptions in `failure_handler`, we tend to print them
so we at least have some echoes of what happened.
test/failure_handler/src/share/classes/jdk/test/failurehandler/ToolKit.java
line 71:
> 69: // just silently skip gzipped core processing
> 70: }
> 71: unpackedCore.toFile().delete();
Suggestion:
Paths.delete(unpackedCore);
``` ?
test/failure_handler/src/share/classes/jdk/test/failurehandler/Utils.java line
73:
> 71: InputStream stream =
> Utils.class.getResourceAsStream(resourceName);
> 72:
> 73: // EFH_CONF_DIR might re-defined to load custom configs for
> development purposes
this also seems to be unrelated to the subject and does require documentation
in `test/failure_handler/README`.
test/failure_handler/src/share/classes/jdk/test/failurehandler/action/PatternAction.java
line 63:
> 61: }
> 62: for (int i = 0, n = args.length; i < n; ++i) {
> 63: args[i] = args[i].replace("%java",
> helper.findApp("java").getAbsolutePath());
are we sure that `java` from `PATH` (which is what `findApp` returns) is the
right `java`?
test/failure_handler/src/share/conf/common.properties line 38:
> 36: jcmd.vm.system_properties \
> 37: jcmd.gc.heap_info jcmd.gc.class_histogram jcmd.gc.finalizer_info \
> 38: jcmd.vm.info \
this seems to be unrelated to the RFE you are working on.
test/failure_handler/src/share/conf/common.properties line 67:
> 65: cores=jhsdb.jstack
> 66: jhsdb.jstack.app=jhsdb
> 67: jhsdb.jstack.args=jstack --mixed --core %p --exe %java
strictly speaking, we can't guarantee that the executable is always `bin/java`,
but it's the most common and the most interesting case, so this assumption is
good enough for our pourposes.
could you please add a comment explaining this so the further reading won't be
puzzled?
test/failure_handler/src/share/conf/mac.properties line 68:
> 66: native.jhsdb.app=bash
> 67: native.jhsdb.jstack.delimiter=\0
> 68: native.jhsdb.jstack.args=-c\0DevToolsSecurity --status | grep -q enabled
> && jhsdb jstack --mixed --pid %p
AFAICS `jhsdb` does check "Developer mode" on macos before trying to attach and
will just report 'lack of privileges' (as opposed to hanging with a modal
window asking for permission), so you can move `jshdb.jstack` to
`common.properties`.
-------------
Changes requested by iignatyev (Reviewer).
PR: https://git.openjdk.java.net/jdk/pull/4234
