RFR: 8207025: JvmtiEnv::SetSystemProperty() does not handle OOM
Ioi Lam
iklam at openjdk.java.net
Mon Mar 28 18:47:49 UTC 2022
On Mon, 28 Mar 2022 06:19:39 GMT, David Holmes <dholmes at 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
>
> Hi Ioi,
>
> I think the real bug in this code is that
>
> `_value = AllocateHeap(strlen(value)+1, mtArguments);`
>
> should be:
>
> `_value = AllocateHeap(strlen(value)+1, mtArguments, AllocFailStrategy::RETURN_NULL);`
>
> as this code is used from `JvmtiEnv::SetSystemProperty` and it seems wrong to abort the VM on that path.
@dholmes-ora To get `JvmtiEnv::SetSystemProperty` working properly, the fix is more complicated. According to the specification
https://docs.oracle.com/en/java/javase/17/docs/specs/jvmti.html#SetSystemProperty
`JvmtiEnv::SetSystemProperty` should return `JVMTI_ERROR_NOT_AVAILABLE` when the property is not available or is not writeable, and JVMTI_ERROR_OUT_OF_MEMORY when OOM (this is one of the "universal errors").
Therefore, I had to change `SystemProperty::set_writeable_value` to return a tri-state result.
I also added the `JVMTI_ERROR_NULL_POINTER` that was in the spec but wasn't implemented.
@sspitsyn could you take a look as well?
-------------
PR: https://git.openjdk.java.net/jdk/pull/7981
More information about the hotspot-runtime-dev
mailing list