RFR(s): 8074860: Structured Exception Catcher missing around CreateJavaVM on Windows
Thomas Stüfe
thomas.stuefe at gmail.com
Tue Mar 17 08:33:41 UTC 2015
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