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

Tomas Kalibera tomas.kalibera at gmail.com
Wed Oct 18 13:12:19 UTC 2017


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