RFR(s): 8074860: Structured Exception Catcher missing around CreateJavaVM on Windows
Thomas Stüfe
thomas.stuefe at gmail.com
Wed Mar 25 16:20:03 UTC 2015
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