[prev in list] [next in list] [prev in thread] [next in thread]
List: openjdk-serviceability-dev
Subject: Re: RFR: 8207025: JvmtiEnv::SetSystemProperty() does not handle OOM
From: Ioi Lam <iklam () openjdk ! java ! net>
Date: 2022-03-29 21:24:47
Message-ID: rB6vy4-KljOs9ZZmrpb6VVdGt1GEzGO6pC1YYkz499o=.f5b11887-7ba2-41fb-b556-0cb83bb0f8e8 () github ! com
[Download RAW message or body]
On Mon, 28 Mar 2022 06:19:39 GMT, David Holmes <dholmes@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.
Thanks @dholmes-ora and @sspitsyn for the review.
-------------
PR: https://git.openjdk.java.net/jdk/pull/7981
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic