RFR: Fix jtreg test runtime/StackGuardPages/

[Siebenborn, Axel]

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
> >>>>>
> >>>>>
> >>>
> >