CSR Review: 8189423: Add option to disable stack overflow checking in primordial thread for use with JNI_CreateJavaJVM

[Thomas Stüfe]

Hi David,

I understand that you want to have a quick solution to a specific problem.
I do not like adding a switch with such a narrow usage and would prefer,
like Robin, a more well rounded and more general solution.

That said, I do not see any specific problems with the proposed switch. Not
even sure if that matters, as I am not even sure I can review CSRs :)

Kind Regards, Thomas


On Tue, Oct 17, 2017 at 3:00 PM, David Holmes <david.holmes at oracle.com>
wrote:

> On 17/10/2017 10:41 PM, Robbin Ehn wrote:
>
>> On 10/17/2017 12:29 PM, David Holmes wrote:
>>
>>> On 17/10/2017 7:28 PM, Robbin Ehn wrote:
>>>
>>>> My understanding from prior discussion (ie when we fixed the 2MB stack
>>>>> limit) is that it doesn't work for them to use a new thread for the JVM.
>>>>> The change to support anything but unlimited stack size also helped (and
>>>>> the 8M limit when 'unlimited' is tolerable) but really they just want a
>>>>> simple way to say "please don't bother with stack guards, I can deal with
>>>>> that myself".
>>>>>
>>>>
>>>> Is there a problem I'm not seeing to extends this to all attaching
>>>> threads?
>>>> E.g. -XX:+DisableAttachingThreadGuardPages (more useful option I would
>>>> say)
>>>>
>>>
>>> Yes - that allows it to impact all of our launchers as well - not
>>> something I want to allow for. It opens the door for a myriad of bug
>>> reports if people use this flag without understanding it's consequences.
>>>
>>> The aim here is to solve a specific problem, not introduce new ways to
>>> break things.
>>>
>>
>> First, since this is broken, you would fix the issue with different
>> stacksizes.
>>
>
> Not sure what you consider "broken". The current approach causes a problem
> for runtimes that want to do their own stack guards. The proposal is to
> allow them by giving a flag to turn ours off - but only for the primordial
> thread, as that is where the problem lies.
>
> And secondly, I think the solution to that is to move all 'text' options
>> to the launcher and have the CreateVM taking a data structure with
>> pre-parsed options. And let the launcher choose which options to expose to
>> the end-user. Meaning our own java launcher would never expose this option.
>> (but this is much bigger discussion)
>>
>
> Let's not start to envisage a complete re-design of how you might
> communicate arguments between a host process and the VM. Please.
>
> Would the option would skip adding guard to any other thread calling
>> CreateVM?
>> If not, have the option really the correct name?
>>
>> E.g. -XX:+DisableCreateVMThreadGuardPages ?
>>
>
> The option would ONLY skip for the PRIMORDIAL thread of the process when
> it loads the VM - hence it has exactly the right name. I specifically do
> not want to allow this for an arbitrary thread that happens to do the
> loading of the VM.
>
> Thanks,
> David
>
>
> Thanks, Robbin
>>
>>
>>> David
>>>
>>> Thanks, Robbin
>>>>
>>>>
>>>>> This is intended to be a very simple, very specific proposal. I
>>>>> suppose it could be flagged as "experimental" if we wanted the flexibility
>>>>> to do something different later. But I don't see that being on the cards.
>>>>>
>>>>> Cheers,
>>>>> David
>>>>>
>>>>>
>>>>>> Thanks Robbin
>>>>>>
>>>>>>
>>>>>>> Kind Regards, Thomas
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> On Tue, Oct 17, 2017 at 9:22 AM, David Holmes <
>>>>>>> david.holmes at oracle.com>
>>>>>>> wrote:
>>>>>>>
>>>>>>> CSR: https://bugs.openjdk.java.net/browse/JDK-8189423
>>>>>>>>
>>>>>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8189170
>>>>>>>>
>>>>>>>> Could I please have a reviewer for this CSR request so I can
>>>>>>>> fast-track it.
>>>>>>>>
>>>>>>>> Comments on the proposal are of course welcome.
>>>>>>>>
>>>>>>>> Thanks,
>>>>>>>> David
>>>>>>>>
>>>>>>>>