RFR(s): 8074860: Structured Exception Catcher missing around CreateJavaVM on Windows
Thomas Stüfe
thomas.stuefe at gmail.com
Fri Mar 27 09:16:53 UTC 2015
Posting this to hotspot-dev in the hope of attracting one more reviewer and
a sponsor.
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-runtime-dev
mailing list