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

[Thomas Stüfe]

Hi David,

On Thu, Mar 12, 2015 at 3:45 AM, David Holmes <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>> 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/>
>>         bug: 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
>>
>>
>>