CSR Review: 8189423: Add option to disable stack overflow checking in primordial thread for use with JNI_CreateJavaJVM
David Holmes
david.holmes at oracle.com
Wed Oct 18 06:58:08 UTC 2017
On 18/10/2017 4:14 PM, Thomas Stüfe wrote:
> Hi David,
>
> On Tue, Oct 17, 2017 at 11:00 PM, David Holmes <david.holmes at oracle.com
> <mailto:david.holmes at oracle.com>> wrote:
>
> 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. :)
>
>
> Sure :)
>
> The main example I was thinking of was the desire to attach native
> threads which have a different stack size than Xss - however, after
> inspecting the code, I see that this point is moot (maybe always was) as
> the VM does already the correct thing by placing the VM guards at the
> end of the real stack size, not at Xss.
Right. Xss is for threads created by the VM - and the launcher also uses
it for the thread that launches the VM and runs main. The relationship
between Xss and the use of the primordial thread to launch the VM has
been where things got messy.**
** IIRC the argument was to artificially make it appear as-if the
primordial thread had the same stack size as all other created threads
so that you wouldn't get in a situation where a computation would pass
on the "main thread" and fail with StackOverflowError on any other
thread. I think it was after this that we just bit the bullet and had
the launcher always kick-off a new thread to become the "main thread".
> The other situations I could think of are cases where the thread stack
> lives in memory which is not protectable, or at least not protectable
> the way hotspot does it, using mprotect. On AIX, that would be the
> primordial thread stack as well as any memory allocated with SystemV shm.
Ok. Is it possible to augment os::uses_stack_guard_pages() to report
false if current thread can't support them ?
> Beside that, I could imagine certain embedders just not wanting the VM
> to place guards, for whatever reason. I could imagine scenarios where
> one wants to pool thread stack memory and does not want that memory
> polluted with guard pages.
I can imagine lots of things too, but we haven't had many (any?) real
requests in 20 years so ... these kinds problems tend to need immediate
solutions so people just work around them.
> But I think now that these cases would best served with an addition to
> the invocation API, by giving JNI_CreateAttachedThread an option to skip
> vm guard creation. That would prevent normal users to fall over a
> misunderstood option. Embedders typically know better what they do.
I agree. If this were to be pursued then a modified attach API would be
the way to do it. Feel free to file an RFE if you'd like to do this. ;-)
> 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]
>
>
> Ah :) Okay then, consider it reviewed :)
If you really want to be a reviewer (totally up to you) please "edit"
the CSR and scroll down to the "reviewed by" box and add your OpenJDK
user name.
I've updated the CSR to make the flag "experimental". Though I need to
check with the R folk to see if that is okay. And added some additional
text about a more general attach API.
Thanks,
David
> ..Thomas
>
> Thanks,
> David
>
> [1] https://wiki.openjdk.java.net/display/csr/CSR+FAQs
> <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>
> <mailto: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>
> <mailto: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>
>
> <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>
>
> <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
>
>
>
More information about the hotspot-runtime-dev
mailing list