RFR(M): 8201375: Add the AllowArchivingWithJavaAgent diagnostic vm option to allow the use of the -javaagent option during CDS dumping
Daniel D. Daugherty
daniel.daugherty at oracle.com
Fri Nov 16 18:52:30 UTC 2018
On 11/16/18 1:18 PM, Ioi Lam wrote:
>
>
> On 11/16/18 9:45 AM, serguei.spitsyn at oracle.com wrote:
>> Hi Calvin,
>>
>> On 11/16/18 09:26, Calvin Cheung wrote:
>>> Serguei, Dan,
>>>
>>> Thanks for taking a look.
>>>
>>> I think the option name is correct but the comment for
>>> Threads::create_vm_init_agents() is incorrect as I believe it will
>>> start both Java native agents. Otherwise, my new tests won't work.
>>
>> It works because there is the system JVMTI (native) agent library
>> libinstrument.so that is supporting java agents.
>> It means that new option first impacts native agents and transitively
>> java agents as well via the libinstrument.so.
>>
>> Dan already asked the question about your original reasoning/intention.
>> If your intention is to control java agent only then the fix is wrong
>> as the real impact is much bigger.
>> If you wanted to control all the agents then the new option name is
>> misleading.
>>
>>
> The intention of this change is:
>
> Disallow all use of native and java agents by default. -- using agents
> during dump time will cause undesirable side effects and make the CDS
> archive unsuitable for production environments.
>
> However, for testing CDS itself, we want to use the Java agent (to run
> arbitrary Java code during dump time, such as causing GC at specific
> times).
>
> As a result, what we are doing is:
>
> + Always disallow non-Java agents.
> + Allow java agents only if AllowArchivingWithJavaAgent is specified.
>
> I think we should clean up the bug title and description if necessary.
> However, the behavior of this patch is what we want. And I think the
> naming of the AllowArchivingWithJavaAgent option is correct (as least
> in the sense of "anything not explicitly allow are strictly forbidden"
> :-)
I'm not sure that a Java agent can exist without the presence of the
backing native agent. For JLI/JPLIS, you have both the Java agent part
and the native agent that supports the Java agent.
So if you're truly disallowing non-Java agents, I'm not sure how that
Java agent can run without the backing native agent. However, this area
may have evolved since I last worked on it...
Dan
>
> Thanks
> - Ioi
>
>
>>> Below is my understanding:
>>>
>>> In thread.cpp:
>>>
>>> 3697 if (Arguments::init_agents_at_startup()) {
>>> 3698 create_vm_init_agents();
>>> 3699 }
>>>
>>> init_agents_at_startup() checks if the _agentList is empty in
>>> arguments.hpp:
>>>
>>> static bool init_agents_at_startup() { return
>>> !_agentList.is_empty(); }
>>>
>>> In arguments.cpp, entries will be added to the _agentList during the
>>> parsing of the -agentLib, -agentPath, and -javaagent arguments. See
>>> lines around 2463-2502.
>>> For the -agentLib and -agentPath args, the _agentList will be
>>> populated via the add_init_agent() function. For the -javaagent, the
>>> _agentList will be populated via the add_instrument_agent() function.
>>>
>>> So in create_vm_init_agents(), it will just loop through the
>>> _agentList and starts all the agents. There's a check for
>>> !agent->is_instrument_agent() at line 4092 which is for not allowing
>>> any JVMTI (native) agent to be run during dump time.
>>>
>>> One more reply to Serguei's comment below...
>>
>> Okay, thank you for the update!
>>
>> Thanks,
>> Serguei
>>
>>>
>>> thanks,
>>> Calvin
>>>
>>> On 11/16/18, 6:08 AM, Daniel D. Daugherty wrote:
>>>> > The flags -agentlib and -agentpath are to run JVMTI (native) agents,
>>>> > and Agent_OnLoad is an entrypoint in native agent library.
>>>>
>>>> The '-agentlib' and '-agentpath' options are used to load and execute
>>>> a native library via the Agent_OnLoad() entry point. However, the
>>>> agent does not have to use JVM/TI (it could be pure JNI):
>>>>
>>>> $ java -help
>>>>
>>>> -agentlib:<libname>[=<options>]
>>>> load native agent library <libname>, e.g.
>>>> -agentlib:hprof
>>>> see also, -agentlib:jdwp=help and
>>>> -agentlib:hprof=help
>>>> -agentpath:<pathname>[=<options>]
>>>> load native agent library by full pathname
>>>>
>>>> The '-javaagent' option is used to load and execute a Java agent:
>>>>
>>>> -javaagent:<jarpath>[=<options>]
>>>> load Java programming language agent, see
>>>> java.lang.instrument
>>>>
>>>> so I agree that the option is misnamed (a bit), but the reasons
>>>> needed to be clarified.
>>>>
>>>> Dan
>>>>
>>>>
>>>> On 11/16/18 1:39 AM, serguei.spitsyn at oracle.com wrote:
>>>>> Hi Calvin,
>>>>>
>>>>> It seems the option name AllowArchivingWithJavaAgent is not right.
>>>>> In fact, it is about JVMTI (native) agents:
>>>>>
>>>>> *// Create agents for -agentlib: -agentpath: and converted -Xrun*
>>>>> *// Invokes Agent_OnLoad*
>>>>> *// Called very early -- before JavaThreads exist*
>>>>> *void Threads::create_vm_init_agents() {*
>>>>> *+ if (DumpSharedSpaces && !AllowArchivingWithJavaAgent) {*
>>>>> *+ vm_exit_during_cds_dumping(*
>>>>> *+ "Must enable AllowArchivingWithJavaAgent in order to run Java
>>>>> agent during CDS dumping");*
>>>>> *+ }*
>>>>> *extern struct JavaVM_ main_vm;*
>>>>> *AgentLibrary* agent;*
>>>>> **
>>>>> *JvmtiExport::enter_onload_phase();*
>>>>> **
>>>>> *for (agent = Arguments::agents(); agent != NULL; agent =
>>>>> agent->next()) {*
>>>>> *+ // CDS dumping does not support native JVMTI agent*
>>>>> *+ if (DumpSharedSpaces && !agent->is_instrument_lib()) {*
>>>>> *+ vm_exit_during_cds_dumping("CDS dumping does not support native
>>>>> JVMTI agent, name", agent->name());*
>>>>> *+ }*
>>>>> *+ *
>>>>> *OnLoadEntry_t on_load_entry = lookup_agent_on_load(agent);*
>>>>> The flags -agentlib and -agentpath are to run JVMTI (native) agents,
>>>>> and Agent_OnLoad is an entrypoint in native agent library.
>>>>> So, I'm thinking if it'd make sense to rename the option to
>>>>> something like below:
>>>>> AllowArchivingWithJVMTIAgent or
>>>>> AllowArchivingWithNativeAgent or
>>>>> AllowArchivingWithAgent
>>>>>
>>>>>
>>>>> http://cr.openjdk.java.net/%7Eccheung/8201375/webrev.00/src/hotspot/share/memory/filemap.cpp.frames.html
>>>>>
>>>>> 212 _allow_archiving_with_java_agent =
>>>>> AllowArchivingWithJavaAgent; ...
>>>>> 1350 if (_allow_archiving_with_java_agent &&
>>>>> !AllowArchivingWithJavaAgent) { 1351
>>>>> FileMapInfo::fail_continue("The setting of the
>>>>> AllowArchivingWithJavaAgent is different " 1352 "from the setting
>>>>> in the shared archive."); 1353 return false; 1354 } The
>>>>> _allow_archiving_with_java_agent is initialized with the
>>>>> AllowArchivingWithJavaAgent at line 212.
>>> The assignment at line 212 is for populating the archive header
>>> during CDS dump time.
>>>>> It is not clear from scratch how these two values can be different
>>>>> at line 1350. At least, some comment explaining it is needed.
>>> The value retrieved from the archive header could be different from
>>> the run time setting.
>>> I will add a comment.
>>>
>>>>> Thanks,
>>>>> Serguei
>>>>>
>>>>>
>>>>> On 11/15/18 10:56, Jiangli Zhou wrote:
>>>>>> +1
>>>>>>
>>>>>> Adding serviceability-dev mailing list. It would be good to have
>>>>>> this reviewed by the JVMTI experts also to make sure all cases
>>>>>> are covered.
>>>>>>
>>>>>> Thanks,
>>>>>>
>>>>>> Jiangli
>>>>>>
>>>>>>
>>>>>> On 11/15/18 9:29 AM, Ioi Lam wrote:
>>>>>>> Hi Calvin,
>>>>>>>
>>>>>>> The changes look good.
>>>>>>>
>>>>>>> Thanks
>>>>>>>
>>>>>>> - Ioi
>>>>>>>
>>>>>>> On 11/14/18 4:40 PM, Calvin Cheung wrote:
>>>>>>>> bug: https://bugs.openjdk.java.net/browse/JDK-8201375
>>>>>>>>
>>>>>>>> webrev: http://cr.openjdk.java.net/~ccheung/8201375/webrev.00/
>>>>>>>>
>>>>>>>> This change is for adding a diagnostic vm option
>>>>>>>> (AllowArchivingWithJavaAgent) to allow the -javaagent in the
>>>>>>>> command line during CDS dumping. Please refer to the
>>>>>>>> corresponding CSR
>>>>>>>> (https://bugs.openjdk.java.net/browse/JDK-8213813) for details.
>>>>>>>>
>>>>>>>> Testing: ran hs-tier{1,2,3} tests through mach5.
>>>>>>>>
>>>>>>>> thanks,
>>>>>>>> Calvin
>>>>>>
>>>>>
>>>>
>>
>
More information about the serviceability-dev
mailing list