RFR(s): 8074860: Structured Exception Catcher missing around CreateJavaVM on Windows

David Holmes david.holmes at oracle.com
Fri Mar 27 12:04:24 UTC 2015


Hi Thomas,

On 27/03/2015 7:16 PM, Thomas Stüfe wrote:
> Posting this to hotspot-dev in the hope of attracting one more reviewer and
> a sponsor.

I can sponsor if required, but am still hoping for someone more 
knowledgeable in Windows SEH to do the review.

David

> Th,ank you,
>
> Thomas
>
>
> On Wed, Mar 25, 2015 at 5:20 PM, Thomas Stüfe <thomas.stuefe at gmail.com>
> wrote:
>
>> ping...
>>
>> I still need a sponsor and another reviewer for this issue. Thank you!
>>
>> Thomas
>>
>> On Wed, Mar 18, 2015 at 11:13 AM, Thomas Stüfe <thomas.stuefe at gmail.com>
>> wrote:
>>
>>> Hi Markus,
>>>
>>> the way to do this cleanly would be to use Vectored Exceptions, which
>>> would give us the same control flow as on Unix signal. However, Vectored
>>> Exception handling was explicitly removed from the hotspot ages ago.
>>> I guess because too many third party libraries install their own VE
>>> handlers.
>>>
>>> To all: I still need one more reviewer and a sponsor. Thanks!
>>>
>>> Thomas
>>>
>>>
>>> On Tue, Mar 17, 2015 at 9:40 AM, Markus Gronlund <
>>> markus.gronlund at oracle.com> wrote:
>>>>
>>>> I don’t think it is worth it. But thanks for thinking about it.
>>>>
>>>>
>>>>
>>>> I was thinking if we could maybe do some callback trickery to set up
>>> the SEH in Windows specific code only inside Threads::create_vm() – but
>>> again, probably not worth it.
>>>>
>>>>
>>>>
>>>> I am ok with the suggestion as it stands.
>>>>
>>>>
>>>>
>>>> /Markus
>>>>
>>>>
>>>>
>>>>
>>>>
>>>> From: Thomas Stüfe [mailto:thomas.stuefe at gmail.com]
>>>> Sent: den 17 mars 2015 09:34
>>>> To: Markus Gronlund
>>>> Cc: hotspot-runtime-dev at openjdk.java.net; David Holmes
>>>>
>>>>
>>>> Subject: Re: RFR(s): 8074860: Structured Exception Catcher missing
>>> around CreateJavaVM on Windows
>>>>
>>>>
>>>>
>>>> Hi Markus, David,
>>>>
>>>>
>>>>
>>>> thanks for reviewing this!
>>>>
>>>>
>>>>
>>>> yes, I also do not like the #ifdefs _WIN32.
>>>>
>>>>
>>>>
>>>> We could pretty it up a bit with macros:
>>>>
>>>>
>>>>
>>>> #define GUARD_SEH_START    __try {
>>>>
>>>> #define GUARD_SEH_END        } __except...
>>>>
>>>>
>>>>
>>>> and defining those empty on non-windows platforms?
>>>>
>>>>
>>>>
>>>> What do you think?
>>>>
>>>>
>>>>
>>>> Kind Regards, Thomas
>>>>
>>>>
>>>>
>>>>
>>>>
>>>>
>>>>
>>>>
>>>>
>>>>
>>>>
>>>>
>>>>
>>>>
>>>>
>>>>
>>>>
>>>>
>>>>
>>>> On Tue, Mar 17, 2015 at 9:24 AM, Markus Gronlund <
>>> markus.gronlund at oracle.com> wrote:
>>>>
>>>> Hi Thomas,
>>>>
>>>> This looks good, thank you for fixing this!
>>>>
>>>> I didn't know that the entire Threads::create_vm() routine is currently
>>> unguarded - interesting.
>>>>
>>>> Small point: I agree with David about the annoyance of having platform
>>> specific #ifdefs in the shared code, but I can't find any other position
>>> where we could solve this better (we still need to reach through to the
>>> ExceptionFilter).
>>>>
>>>> Let me know when you start to dig into the SEH (or lack of) for
>>> attaching threads :-)
>>>>
>>>> Thanks again
>>>> Markus
>>>>
>>>>
>>>> -----Original Message-----
>>>> From: Thomas Stüfe [mailto:thomas.stuefe at gmail.com]
>>>> Sent: den 16 mars 2015 12:32
>>>> To: David Holmes
>>>> Cc: hotspot-runtime-dev at openjdk.java.net
>>>> Subject: Re: RFR(s): 8074860: Structured Exception Catcher missing
>>> around CreateJavaVM on Windows
>>>>
>>>> Hi,
>>>>
>>>> I still need one or two reviewers and a sponsor.
>>>>
>>>> Thank you!
>>>>
>>>> Thomas
>>>>
>>>> On Thu, Mar 12, 2015 at 10:41 AM, David Holmes <david.holmes at oracle.com
>>>>
>>>> wrote:
>>>>
>>>>> Hi Thomas,
>>>>>
>>>>> Thanks for the added info. I have no further comments. Hopefully
>>>>> someone with SEH knowledge will also respond.
>>>>>
>>>>> David
>>>>>
>>>>> On 12/03/2015 7:18 PM, Thomas Stüfe wrote:
>>>>>
>>>>>> Hi David,
>>>>>>
>>>>>> On Thu, Mar 12, 2015 at 3:45 AM, David Holmes
>>>>>> <david.holmes at oracle.com <mailto:david.holmes at oracle.com>> wrote:
>>>>>>
>>>>>>      On 12/03/2015 8:03 AM, Thomas Stüfe wrote:
>>>>>>
>>>>>>          Hi David,
>>>>>>
>>>>>>          On Wed, Mar 11, 2015 at 9:43 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:
>>>>>>
>>>>>>               Hi Thomas,
>>>>>>
>>>>>>               I'm not really familiar with Windows SEH. Won't this
>>> break
>>>>>>          custom
>>>>>>               launchers that already provide their own try/catch
>>> around
>>>>>>          Crate_JavaVM ?
>>>>>>
>>>>>>
>>>>>>          No. Windows SEH works stack based: any exception - e.g. a
>>> crash -
>>>>>>          between __try and __except will be handled by the handler
>>> given
>>>>>>          in the
>>>>>>          __except clause. Because they are stack based, they can be
>>>>>>          nested. If an
>>>>>>          exception is raised inside the inner __try/__except, first
>>>>>> the inner
>>>>>>          handler is used, if that one feels not responsible, the next
>>>>>>          outer one
>>>>>>          and so on.
>>>>>>
>>>>>>          With my fix, any exception raised inside CreateJavaVM will be
>>>>>>          handler by
>>>>>>          our handler topLevelExceptionFilter; only if our handler
>>> feels not
>>>>>>          responsible (returns EXCEPTION_CONTINUE_SEARCH), the user
>>>>>>          handler will
>>>>>>          be called.
>>>>>>
>>>>>>
>>>>>>      My lack of knowledge about when our handler "feels responsible"
>>>>>>      still leaves me a little nervous here. :)
>>>>>>
>>>>>>
>>>>>> I think the patch is quite safe. I added this patch to our code base
>>>>>> in
>>>>>> 2011 and since then this is active in productive code for SAP
>>> customers.
>>>>>> The SAP jvm gets heavily used with custom launchers which do their
>>>>>> own error handling, so this is a well tested scenario.
>>>>>>
>>>>>> I would like to get a similar signal handling coverage as on UNIX:
>>>>>>
>>>>>> On Unix, we have global signal handling. The moment signal handling
>>>>>> is established early in os::init(), every signal from everywhere is
>>>>>> covered, even user code. We even have to take care that user handlers
>>>>>> get also in the loop via signal chaining, libjsig.so etc.
>>>>>>
>>>>>> On Windows, it is the other way around: we have stack based signal
>>>>>> handling , so we need __try/__except on every thread, and this means
>>>>>> there are parts of jvm code which run without signal handling:
>>>>>> - the whole initialization
>>>>>> - attached threads (I think?)
>>>>>> which means that on those cases, user handler gets signals which the
>>>>>> libjvm should handle.
>>>>>>
>>>>>> This was "fixed" partly by surrounding small code which we know
>>>>>> beforehand causes signals - how convenient - with __try/__except. For
>>>>>> example, the code which handles "-XX:ErrorHandlerTest" to trigger a
>>>>>> crash. But you want error handling to always work. I also do not know
>>>>>> if stuff like polling pages, implicit nulltests etc could be used in
>>>>>> unprotected code.
>>>>>>
>>>>>> As a side note, there is a UNIX-like signal handling mode on Windows
>>>>>> too, "vectored exception handling", which was used in the jvm but
>>>>>> removed some time ago for reasons I do not really know.
>>>>>>
>>>>>>          Any exception raised in the launcher itself outside of
>>>>>>          CreateJavaVM will
>>>>>>          still be handled by the user handler.
>>>>>>
>>>>>>               (And I hate seeing the win32 ifdefs in the shared code
>>> :( ).
>>>>>>
>>>>>>
>>>>>>          Yes I know, I kind of expected that feedback :( - I did not
>>> find a
>>>>>>          better way of doing this. One could try to hide the
>>> __try/__except
>>>>>>          behind macros, but that would be kind of unwieldy and I
>>> don't like
>>>>>>          abstractiing something which only has meaning on one
>>> platform.
>>>>>>
>>>>>>
>>>>>>      Does it help if we make the caller responsible for SEH and then
>>> put
>>>>>>      the try/catch in the launcher code (hopefully in a windows
>>> specific
>>>>>>      part thereof) ?
>>>>>>
>>>>>>
>>>>>> No, because the caller would need access to "topLevelExceptionFilter"
>>>>>> - you would need to export that function from the libjvm and then
>>>>>> tell the caller "always call topLevelExceptionFilter() if a signal
>>>>>> happens on Windows", which is quite awkward and different than on
>>> UNIX.
>>>>>>
>>>>>> Thomas
>>>>>>
>>>>>>      Thanks,
>>>>>>      David
>>>>>>
>>>>>>
>>>>>>               Thanks,
>>>>>>               David
>>>>>>
>>>>>>
>>>>>>
>>>>>>          Kind regards, Thomas
>>>>>>
>>>>>>               On 12/03/2015 1:40 AM, Thomas Stüfe wrote:
>>>>>>
>>>>>>                   Hi all,
>>>>>>
>>>>>>                   please review this smallish change:
>>>>>>
>>>>>>                   webrev:
>>>>>>          http://cr.openjdk.java.net/~____stuefe/webrevs/8074860/
>>>>>> webrev.____01/webrev/
>>>>>>          <http://cr.openjdk.java.net/~__stuefe/webrevs/8074860/
>>>>>> webrev.__01/webrev/>
>>>>>>
>>>>>>          <http://cr.openjdk.java.net/~__stuefe/webrevs/8074860/
>>>>>> webrev.__01/webrev/
>>>>>>          <http://cr.openjdk.java.net/~stuefe/webrevs/8074860/webrev.
>>>>>> 01/webrev/>>
>>>>>>                   bug:
>>>>>>          https://bugs.openjdk.java.net/____browse/JDK-8074860
>>>>>>          <https://bugs.openjdk.java.net/__browse/JDK-8074860>
>>>>>>
>>>>>>                   <https://bugs.openjdk.java.__net/browse/JDK-8074860
>>>>>>
>>>>>>          <https://bugs.openjdk.java.net/browse/JDK-8074860>>
>>>>>>
>>>>>>                   This change adds SEH guards around
>>> JNI_CreateJavaVM().
>>>>>>          Without
>>>>>>                   the change,
>>>>>>                   on Windows, the VM initialization runs without crash
>>>>>>          protection:
>>>>>>                   crashes
>>>>>>                   will terminate VM immediately without writing an
>>>>>> error log;
>>>>>>                   also, any
>>>>>>                   techniques relying on signals will not work, e.g.
>>>>>>          SafeFetch().
>>>>>>
>>>>>>                   This was partly solved before on a case-by-case
>>> base by
>>>>>>          wrapping
>>>>>>                   code
>>>>>>                   sections which may crash in their own __try/__except
>>>>>>          wrappers -
>>>>>>                   e.g. CPU
>>>>>>                   feature probing.
>>>>>>
>>>>>>                   The change guards the whole of JNI_CreateJavaVM
>>>>>>          invocation in
>>>>>>                   __try/__except. Unfortunately, for that to compile,
>>> I
>>>>>>          needed to
>>>>>>                   introduce a
>>>>>>                   wrapper around JNI_CreateJavaVM and move the whole
>>> of
>>>>>>                   JNI_CreateJavaVM to a
>>>>>>                   new function "JNI_CreateJavaVM_inner".
>>>>>>
>>>>>>                   This fix also gets rid of various workarounds which
>>>>>>          were used
>>>>>>                   before to
>>>>>>                   guard code sections.
>>>>>>
>>>>>>                   Thanks for reviewing!
>>>>>>
>>>>>>                   Oh, on a side note: I tried to figure out if threads
>>>>>>          which are
>>>>>>                   attached
>>>>>>                   from the outside via JNI AttachCurrentThread() are
>>> in
>>>>>>          any way
>>>>>>                   guarded with
>>>>>>                   SEH protection. Newly created threads are guarded
>>>>>>          because they
>>>>>>                   run thru
>>>>>>                   java_start() in os_windows.cpp, which adds SEH
>>> guards
>>>>>>          to all
>>>>>>                   frames below.
>>>>>>                   But for attached threads, I did not find any SEH
>>> guards
>>>>>>          - or
>>>>>>                   maybe I am
>>>>>>                   blind? What does that mean for java code running
>>> inside
>>>>>>          attached
>>>>>>                   threads?
>>>>>>
>>>>>>                   Regards,
>>>>>>
>>>>>>                   Thomas Stuefe
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>
>>>>
>>>
>>
>>


More information about the hotspot-runtime-dev mailing list