RFR: Fix jtreg test runtime/StackGuardPages/

Mon Apr 9 10:33:08 UTC 2018

Hi Mikael,
I attached the patch.

Thanks,
Axel

> -----Original Message-----
> From: Mikael Vidstedt [mailto:mikael.vidstedt at oracle.com]
> Sent: Freitag, 6. April 2018 19:52
> To: Siebenborn, Axel <axel.siebenborn at sap.com>
> Cc: David Holmes <david.holmes at oracle.com>; portola-
> dev at openjdk.java.net
> Subject: Re: RFR: Fix jtreg test runtime/StackGuardPages/
> 
> 
> Looks good, thanks for making the changes!
> 
> Send to me and I’ll push.
> 
> Cheers,
> Mikael
> 
> > On Apr 6, 2018, at 1:23 AM, Siebenborn, Axel <axel.siebenborn at sap.com>
> wrote:
> >
> > Hi David,
> > You're right, it's better not to use a *_np function.
> >
> > I prepared a new webrev:
> >
> http://cr.openjdk.java.net/~asiebenborn/jtreg_StackGuardPages/webrev_c
> /
> >
> > Thanks,
> > Axel
> >
> >> -----Original Message-----
> >> From: David Holmes [mailto:david.holmes at oracle.com]
> >> Sent: Freitag, 6. April 2018 09:13
> >> To: Siebenborn, Axel <axel.siebenborn at sap.com>; Mikael Vidstedt
> >> <mikael.vidstedt at oracle.com>
> >> Cc: portola-dev at openjdk.java.net
> >> Subject: Re: RFR: Fix jtreg test runtime/StackGuardPages/
> >>
> >> On 6/04/2018 4:41 PM, Siebenborn, Axel wrote:
> >>> Hi,
> >>> As discussed off-list with Mikael, I get a reasonable stack size for
> >> pthread_create by using JNI_GetDefaultJavaVMInitArgs.
> >>> This way, we have a stack suitable to run java code.  The stacksize  is not
> >> hardcoded in the test and platform independent.
> >>
> >> The test isn't platform independent when it uses a *_np function.
> >>
> >> I'm not sure why you are changing the default attributes used by
> >> pthread_create instead of just passing a suitable attr object when
> >> creating the new thread ref. pthread_attr_setstacksize ??
> >>
> >> Cheers,
> >> David
> >>
> >>> I updated the webrev:
> >>>
> >>
> http://cr.openjdk.java.net/~asiebenborn/jtreg_StackGuardPages/webrev_a
> >> /
> >>>
> >>> Regards,
> >>> Axel
> >>>
> >>>> -----Original Message-----
> >>>> From: Mikael Vidstedt [mailto:mikael.vidstedt at oracle.com]
> >>>> Sent: Dienstag, 3. April 2018 19:31
> >>>> To: Siebenborn, Axel <axel.siebenborn at sap.com>
> >>>> Cc: portola-dev at openjdk.java.net
> >>>> Subject: Re: RFR: Fix jtreg test runtime/StackGuardPages/
> >>>>
> >>>>
> >>>> The total stack size is 80kb, of which 40kb/10 pages worth is used for
> >> shadow
> >>>> pages (48kb/12 pages in debug mode). That leaves 80-48=32kb stack
> for
> >>>> execution, which admittedly is not a lot in today’s world. However, I
> >> would
> >>>> have thought that the initialization would still have room to play in that,
> >> but it
> >>>> seems not?
> >>>>
> >>>> I also assume that the reason why this isn’t a problem for the normal
> >>>> launcher case is that the launcher doesn’t use the primordial. Perhaps
> the
> >>>> right way of fixing this is buy having the test specific launcher here
> mimic
> >>>> what the real launcher does (something not entirely unlike
> >>>> ContinueInNewThread)?
> >>>>
> >>>> Cheers,
> >>>> Mikael
> >>>>
> >>>>> On Apr 3, 2018, at 5:30 AM, Siebenborn, Axel
> >> <axel.siebenborn at sap.com>
> >>>> wrote:
> >>>>>
> >>>>> Hi Mikael,
> >>>>> Both  tests fails because
> >>>>> res = (*_jvm)->AttachCurrentThread(_jvm, (void **)&env, NULL);
> >>>>> returns with an error (res != JNI_OK).
> >>>>>
> >>>>> AttachCurrentThread uses the current thread as JavaThread and
> >> allocates a
> >>>> java level Thread object and needs to run initializing java code
> >>>> (JavaThread::allocate_threadObj).
> >>>>> In order to run java code, the remaining stack space is checked. There
> >> must
> >>>> be sufficient  space for an interpreter frame, the java frame and
> shadow
> >>>> pages ( JavaCalls::call_helper() ). The default for the number of shadow
> >>>> pages is 10 for a release build. If this check fails a
> StackOverflowException
> >> is
> >>>> thrown.
> >>>>> As we return with a pending exception the attach fails and we return
> >>>> JNI_ERR.
> >>>>>
> >>>>> This is a problem as we call AttachCurrentThread from a thread that
> we
> >>>> created with the default stacksize. Threads created by the jvm are
> >> created
> >>>> with a much higher stacksize.
> >>>>>
> >>>>> webrev_a fixes the test by providing an argument for the stacksize to
> >>>> pthread_create.
> >>>>>
> >>>>> webrev_b reduces the amount of shadow pages.
> >>>>>
> >>>>> Does that make things clearer?
> >>>>>
> >>>>> Regards,
> >>>>> Axel
> >>>>>
> >>>>>
> >>>>>> -----Original Message-----
> >>>>>> From: Mikael Vidstedt [mailto:mikael.vidstedt at oracle.com]
> >>>>>> Sent: Dienstag, 3. April 2018 00:01
> >>>>>> To: Siebenborn, Axel <axel.siebenborn at sap.com>
> >>>>>> Cc: portola-dev at openjdk.java.net
> >>>>>> Subject: Re: RFR: Fix jtreg test runtime/StackGuardPages/
> >>>>>>
> >>>>>>
> >>>>>> Alex,
> >>>>>>
> >>>>>> Can you please add some additional color/details to why it fails? Is it
> the
> >>>>>> native or java variant that fails (or both)?
> >>>>>>
> >>>>>> I understand that the stack is smaller with musl (I ran into that
> problem
> >> in
> >>>> a
> >>>>>> different context), but I’m not sure I see why Thread.init() would fail
> >> just
> >>>>>> because of that. Help? :)
> >>>>>>
> >>>>>> Cheers,
> >>>>>> Mikael
> >>>>>>
> >>>>>>> On Mar 22, 2018, at 3:37 AM, Siebenborn, Axel
> >>>>>> <axel.siebenborn at sap.com> wrote:
> >>>>>>>
> >>>>>>> Hi,
> >>>>>>>
> >>>>>>> the test  runtime/StackGuardPages/ fails, as AttachCurrentThread()
> >>>> returns
> >>>>>> with an error. The reason is a stackoverflow while trying to run java
> >>>>>> Thread.init().
> >>>>>>> The default pthread stack size is 80 K, much less than on other
> Unixes.
> >>>>>>>
> >>>>>>> The first fix is to check the pthread default stacksize and increase it
> if
> >> it's
> >>>>>> less than 160K.
> >>>>>>>
> >>>>>>> Webrev a:
> >>>>>>>
> >>>>>>
> >>>>
> >>
> http://cr.openjdk.java.net/~asiebenborn/jtreg_StackGuardPages/webrev_a
> >>>>>> /
> >>>>>>>
> >>>>>>> However, the reason we need that much stack, just to run a single
> >> java
> >>>>>> method, is the large amount of shadow pages.
> >>>>>>> The second fix is just to add a flag to JNI_CreateJavaVM, to set the
> >>>> number
> >>>>>> of shadow pages to a lower value. However, a change in
> >> globals_x86.hpp
> >>>> is
> >>>>>> needed to allow smaller minimum for that value. Here I miss a macro
> >> like
> >>>>>> MUSL_OLNLY(code).
> >>>>>>>
> >>>>>>> Webrev b:
> >>>>>>>
> >>>>>>
> >>>>
> >>
> http://cr.openjdk.java.net/~asiebenborn/jtreg_StackGuardPages/webrev_
> >>>>>> b/
> >>>>>>>
> >>>>>>> I'm unsure which change to use.
> >>>>>>> What do you think?
> >>>>>>>
> >>>>>>> Regards,
> >>>>>>> Axel
> >>>>>>>
> >>>>>>>
> >>>>>
> >>>