Just read Matthias reply:
We call jplis_assert() if allocation fails. Looking at
src/java.instrument/share/native/libinstrument/JPLISAssert.h
I see that these assertions seem to be turned on all the time:
45 #define JPLISASSERT_ENABLEASSERTIONS (1)
and lands us in JPLISAssertCondition() (possible improvement here is to
evaluate the condition before the call):
58 #define jplis_assert(x) JPLISAssertCondition((jboolean)(x),
#x, THIS_FILE, __LINE__)
However, JPLISAssertCondition() is not an assert - name is misleading - but
just a printf():
39 void
40 JPLISAssertCondition( jboolean condition,
41 const char * assertionText,
42 const char * file,
43 int line) {
44 if ( !condition ) {
45 fprintf(stderr, "*** java.lang.instrument ASSERTION FAILED ***:
\"%s\" at %s line: %d\n",
46 assertionText,
47 file,
48 line);
49 }
50 }
Maybe I miss something but I do not see an abort.
----
I think we should add an exit(2) or abort(2) to the assertion.
But I also think this is a different issue from what Matthias tries to fix,
so I am fine with Matthias change.
Cheers, Thomas
On Fri, Nov 29, 2019 at 8:20 AM Alan Bateman <alan.bate...@oracle.com>
wrote:
> On 29/11/2019 06:29, Thomas Stüfe wrote:
> > Hi Matthias,
> >
> > I am not certain the callers are prepared to handle NULL.
> >
> > This is used in a chain of TRANSFORM macro calls which AFAICS do not
> > handle NULL; e.g. , at 872, we pass the returned pointer to
> > convertUft8ToPlatformString which passes it on (on Windows) to
> > MultiByteToWideChar, which does not handle NULL input.
> >
> > So I wonder whether a clear error message with an exit would be better
> > in this case. Otherwise we may get a crash just some instructions later.
> >
> Right, this needs a lot more analysis to see if it's even possible to
> continue. The main usage is VM startup where the -javaagent option
> specifies agents that have the Boot-Class-Path attribute. In that case
> it would not be unreasonable to abort the process, it's unlikely to get
> much startup in the startup if memory is exhausted. The other possible
> context is where a tool agent is loaded into a running VM, in which case
> have the attach thread return with a pending exception might be okay
> (although the VM is likely to shutdown anyway as the memory exhaustion
> will be detected/handled elsewhere).
>
> -Alan
>
