On Tue, 23 May 2023 15:32:58 GMT, Alan Bateman <al...@openjdk.org> wrote:
>> This is the implementation for JEP 451. There are two parts to this: >> >> 1. A multi-line warning is printed when a JVM TI or Java agent is loaded >> into a running VM. For JVM TI, the message is printed to stderr from >> JvmtiAgent::load. For Java agents, it is printed to System.err (as that may >> be redirected) in the JPLIS (j.l.instrumentation) implementation. This part >> includes an update to the JVM TI spec and API docs to require the warning. >> >> 2. If running with -Djdk.instrument.traceUsage or >> -Djdk.instrument.traceUsage=true, the calls to the Instrumentation API print >> a trace message and stack trace. > > Alan Bateman has updated the pull request with a new target base due to a > merge or a rebase. The incremental webrev excludes the unrelated changes > brought in by the merge/rebase. The pull request contains 12 additional > commits since the last revision: > > - Tweak javadoc, update test to use more test infra > - Merge > - Merge > - Refresh package description > - Merge > - Tweak docs > - Merge > - Draft docs changes > - Merge > - Rename/cleanup > - ... and 2 more: https://git.openjdk.org/jdk/compare/23703312...87022831 When testing the proposed patch, I stumbled upon two (related) issues: 1. There are no checks that an agent have been already loaded. When I invoke `agent_load` multiple times with exactly the same library path, the warning is printed every time. The repeated warnings are not helpful; they are neither good for user experience nor for integrity, since agent developers will be tempted to suppress excessive warnings on the first load by hacking the JDK internals and thus breaking integrity even more. 2. If an agent is loaded at JVM startup using `-agentlib/agentpath` option, the subsequent communication with the agent through jcmd results in the warning being printed again and again. This contradicts the intentions described in JEP 451: > Tools that employ agents loaded at startup are unaffected by these changes. As @pron mentioned, the presence of `-agentlib/agentpath` option serves as an explicit user consent to use the tool, and disallowing such agents (or issuing a warning about them) is a non-goal of the JEP. With the current behavior, users will be tempted to add `-XX:+EnableDynamicAgentLoading` option instead of putting one particular agent on the command line. This again does not serve the purpose of strengthen the integrity. The use cases I mentioned here are quite natural and supported by popular profilers: a user may want to use a tool some time after the JVM startup, or reconfigure it later at runtime similarly to how `jcmd JFR.configure` works. ------------- Changes requested by apangin (no project role). PR Review: https://git.openjdk.org/jdk/pull/13899#pullrequestreview-1447064684
