[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-serviceability-dev
Subject:    Re: RFR [XS]: 8234968: check calloc rv in libinstrument InvocationAdapter
From:       Alan Bateman <Alan.Bateman () oracle ! com>
Date:       2019-11-29 10:41:08
Message-ID: 5ca42824-37d8-44d5-7d49-8b270852d2e6 () oracle ! com
[Download RAW message or body]



On 29/11/2019 07:32, Thomas Stüfe wrote:
> 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.
>
There is technical debt that dates back to the original development of 
the JPLIS agent in JDK 5. If we run out of memory during VM startup then 
doing a graceful abort seems right, pointless trying to continue. I 
don't know how far you want to go with the current patch but a bit icky 
to continue (even with a warning) with the wrong configuration. The late 
binding agent case is different of course, I think the native method has 
to complete with a pending exception rather than aborting the VM.

-Alan
[prev in list] [next in list] [prev in thread] [next in thread]