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

[David Holmes]

Hi Thomas,

On 18/10/2017 1:11 AM, Thomas Stüfe wrote:
> 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.

To have a "more well rounded and more general solution" you first need a 
more general problem. :) I'm not at all clear what problem the ability 
to control stack guards for any attaching thread would be solving.

> That said, I do not see any specific problems with the proposed switch. 

Thank you.

> Not even sure if that matters, as I am not even sure I can review CSRs :)

You certainly can!

Q: Who should be a reviewer on a CSR proposal?
A: One or more engineers with expertise in the areas impacted by the 
proposed change should review the CSR request and be listed as a 
reviewer before the proposal is reviewed by the CSR membership. (These 
engineers may or may not be Reviewers on the corresponding JDK project.) [1]

Thanks,
David

[1] https://wiki.openjdk.java.net/display/csr/CSR+FAQs

> Kind Regards, Thomas
> 
> 
> On Tue, Oct 17, 2017 at 3:00 PM, David Holmes <david.holmes at oracle.com 
> <mailto: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
>                             <mailto:david.holmes at oracle.com>>
>                             wrote:
> 
>                                 CSR:
>                                 https://bugs.openjdk.java.net/browse/JDK-8189423
>                                 <https://bugs.openjdk.java.net/browse/JDK-8189423>
> 
>                                 Bug:
>                                 https://bugs.openjdk.java.net/browse/JDK-8189170
>                                 <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
> 
>