CSR Review: 8189423: Add option to disable stack overflow checking in primordial thread for use with JNI_CreateJavaJVM
Thomas Stüfe
thomas.stuefe at gmail.com
Wed Oct 18 08:05:30 UTC 2017
Hi David,
On Wed, Oct 18, 2017 at 8:58 AM, David Holmes <david.holmes at oracle.com>
wrote:
> 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".
>
>
I see.
> 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 ?
>
>
os::uses_stack_guard_pages() is not necessarily used from the same thread
(e.g. cpu/x86/frame_x86.cpp, frame::safe_for_sender()).
btw, if we do not augment it, this could be a candidate for cleaning up,
no? All platforms just return "true".
> 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'll do if I have spare time :)
> 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.
>
>
Okay, I did add myself as reviewer.
..Thomas
> 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