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

[David Holmes]

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
>
>
>