On Mon, 28 Mar 2022 18:34:31 GMT, Ioi Lam <ik...@openjdk.org> wrote:
>> `JvmtiEnv::SetSystemProperty` eventually calls `PathString::set_value` in
>> arguments.cpp, which aborts the VM when it fails to allocate a string copy
>> of the property value.
>>
>>
>> bool PathString::set_value(const char *value) {
>> if (_value != NULL) {
>> FreeHeap(_value);
>> }
>> _value = AllocateHeap(strlen(value)+1, mtArguments );
>> // should pass AllocFailStrategy::RETURN_NULL -----^
>> assert(_value != NULL, "Unable to allocate space for new path value");
>>
>>
>> This should be fixed so that `JvmtiEnv::SetSystemProperty` can return
>> `JVMTI_ERROR_OUT_OF_MEMORY` in case of OOM. See
>> https://docs.oracle.com/en/java/javase/17/docs/specs/jvmti.html#SetSystemProperty
>
> Ioi Lam has updated the pull request incrementally with one additional commit
> since the last revision:
>
> @dholmes-ora comments: changed implementation to work with
> JvmtiEnv::SetSystemProperty
Hi Ioi,
I don't think this needs to be that complicated. In set_system_property you can
query if the flag is writeable() directly, so then you don't need a ternary
return value from set_property_value().
Also see comment below.
David
src/hotspot/share/prims/jvmtiEnv.cpp line 3431:
> 3429: jvmtiError
> 3430: JvmtiEnv::SetSystemProperty(const char* property, const char*
> value_ptr) {
> 3431: NULL_CHECK(property, JVMTI_ERROR_NULL_POINTER);
You don't need this. The null check happens in the wrapper code generated from
the xml file. See
build/<config>/hotspot/variant-server/gensrc/jvmtifiles/jvmtiEnterTrace.cpp
-------------
Changes requested by dholmes (Reviewer).
PR: https://git.openjdk.java.net/jdk/pull/7981
