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 21:57:13 UTC 2017
Thanks Tomas!
David
On 18/10/2017 11:12 PM, Tomas Kalibera wrote:
>
> Hi David,
>
> On 10/18/2017 08:58 AM, david.holmes at oracle.com (David Holmes) 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".
>>
>>> 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.
> The proposal looks good to me and I think is a clean and simple solution
> to the problem that Java is reducing our stack on the R thread (R is
> single threaded). With this new option, we would have to worry neither
> about -Xss/ThreadStackSize reducing our stack nor about various caps
> like the 2M/Linux in the past or the 8M for unlimited stack now. To me
> the option seems potentially useful also to any other language runtime
> that is embedding Java.
>
> Thanks
> Tomas
>
>
>
>>
>> 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