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-dev
mailing list