RFR: Fix jtreg test runtime/StackGuardPages/

[David Holmes]

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