On Mon, 28 Mar 2022 06:19:39 GMT, David Holmes <dhol...@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