Hi Sergey,
This bug happens, when original "opposite" is HWND of the peer object of
the "java.awt.Component", like "java.awt.Label", "java.awt.TextArea"
etc. which are strictly not instances of "java.awt.Window" class. These
AWT components cannot be displayed on a screen without being located in
some instance of "java.awt.Window". Therefore getting parent HWND of the
peer's HWND of these AWT components works and resolves the bug. This
approach resolves the bug also because there is a reproducer of this bug
which you saw yourself and which proves that the fix resolves the crash.
This is the reply to your question "why it will work fine at step(2)?".
When JDK is used in a normal way and not as it is used in the
reproducer, "opposite" is always HWND of a peer of "java.awt.Window"
instance, thus for such cases there is no need to search for some parent
HWND of the "opposite" HWND. I do not want to break behavior of JDK for
these normal scenarios, by getting some parent HWND of the valid
"opposite" HWND. I do not want to rely on expectation that for valid
"opposite" HWND the method "AwtComponent::GetTopLevelParentForWindow"
must return the same "opposite" HWND, I do not want to rely on this
expectation and run the risk of using some incorrect HWND as opposite,
when there was already valid "opposite" HWND in hands, and by this to
break existing JDK behavior related to focus handling, because this
edited "AwtWindow::SendWindowEvent" method is really involved in focus
handling.
Thank you,
Anton
On 15/04/2020 07:56, Sergey Bylokhov wrote:
On 4/14/20 11:12 am, Anton Litvinov wrote:
I think that it is better not to change current well established old
behavior in the method "AwtWindow::SendWindowEvent(jint, HWND, jint,
jint)" - what is first to get "jobject" from the original "opposite"
HWND and try to use it for construction of "TimedWindowEvent". I was
trying to make a fix which will just extend the code and will address
just this very unreal and very narrow scenario without running the
risk of affecting or breaking wide range of other normal scenarios
under which this code was working well from the moment of its
creation. If we assign parent HWND to the "opposite" from the very
beginning, then we will change the currently existing behavior in
favor of addressing this almost unreal bug and may for some cases,
which we do not know yet, start using parent HWND instead of a valid
original "opposite" HWND whose target will be instance of
"java.awt.Window".
In my opinion it is better not to omit first attempt to use original
"opposite" with related to it "jobject" instance.
If it is not safe to replace the opposite by the
GetTopLevelParentForWindow(1) in the first place then
why it is safe to use it in the fix later(1)? What could go wrong at
step (1) and why it will work fine
at step(2)?
Thank you,
Anton
On 14/04/2020 16:41, Sergey Bylokhov wrote:
Hi, Anton.
Probably it is possible to simplify the code a little bit:
Can we just replace initial "opposite" by the top level HWND?
+ opposite = AwtComponent::GetTopLevelParentForWindow(opposite);
AwtComponent *awtOpposite = AwtComponent::GetComponent(opposite);
and add only one check
jOpposite = awtOpposite->GetTarget(env);
+ if ((jOpposite != NULL) &&
+ !env->IsInstanceOf(jOpposite, windowCls)) {
+ env->DeleteLocalRef(jOpposite);
+ jOpposite = NULL;
+ }
On 4/10/20 1:32 pm, Anton Litvinov wrote:
Hello,
Could you please review the following fix for the bug.
Bug: https://bugs.openjdk.java.net/browse/JDK-8242498
Webrev: http://cr.openjdk.java.net/~alitvinov/8242498/jdk15/webrev.00
The bug is the JVM crash, which occurs because a not existing
method is called on a Java object which is not an instance of the
expected Java class that has such a method. Such discrepancy of the
expected type and the type in runtime is possible, because the Java
object, whose field value is set to the instance of the not
expected Java class, is instantiated by AWT native code through JNI
invocation. Since JNI does not validate arguments passed to Java
class constructor and since AWT native code does not validate
arguments prior to invoking Java class constructor through JNI,
such invalid object is created.
REASON OF THE CRASH:
The fact that in the method
"java.awt.DefaultKeyboardFocusManager.dispatchEvent(AWTEvent)" in
the case "WindowEvent.WINDOW_LOST_FOCUS" of switch operator the
variable defined by the expression "Window oppositeWindow =
we.getOppositeWindow();" in runtime is instance of
"java.awt.Component" class instead of "java.awt.Window" class. The
crash occurs during attempt to call the method
"java.awt.Window.getTemporaryLostComponent()" on the object
"oppositeWindow" which in runtime is "Component" instead of the
expected "Window" object, and since the method
"getTemporaryLostComponent()" does not exist in
"java.awt.Component" class JVM cannot find this method and
initiates the crash.
ROOT CAUSE OF THE BUG:
Transfer of the object of the incompatible type
"java.awt.Component" instead of an object of "java.awt.Window" type
as "opposite" argument to the constructor "TimedWindowEvent(Window
source, int id, Window opposite, int oldState, int newState, long
time)" of the class "sun.awt.TimedWindowEvent" through JNI
invocation. This JNI invocation occurs in the C++ class method
"AwtWindow::SendWindowEvent(jint, HWND, jint, jint)" in the file
"src/java.desktop/windows/native/libawt/windows/awt_Window.cpp".
The exact expression creating the instance of Java class
"TimedWindowEvent" with the invalid value of the field "opposite"
is following:
jobject event = env->NewObject(wClassEvent, wEventInitMID, target, id,
jOpposite, oldState, newState,
::JVM_CurrentTimeMillis(NULL, 0));
THE FIX:
The fix changes "AwtWindow::SendWindowEvent(jint, HWND, jint,
jint)" method in the file "awt_Window.cpp" to introduce the code
which verifies that the Java object "jOpposite" is really instance
of the class "java.awt.Window", and if it is not then the fix tries
to get Java object corresponding to parent window of the original
"opposite" HWND. And if this parent window object also is not
instance of "java.awt.Window" class, then NULL value is passed to
the constructor of "TimedWindowEvent" class.
Thank you,
Anton
