Bug 543747 - [win32] JVM crash after connecting Windows Remote Desktop

Changes
-------
1) ImmAssociateContext(0) replaced with ImmAssociateContext(oldContext)
   This avoids Windows bug in MSCTF.DLL
2) Refactoring: Shell no longer inherits HIMC through
   Control.createHandle() before replacing it with a new HIMC in
   Shell.createHandle().
   This probably didn't cause any problems.
3) Refactoring: Connection between HIMC in Control and Shell is more
   visible now.
4) Added win32 manual tests, similar to GTK.

Internal map is not cleaned properly
------------------------------------
1) 'TextInputFramework!TextInputClient' contains an internal std::map
   This map keeps track (through multiple wrappers) of input contexts.
2) For 'Edit' controls, Windows uses different input context
   'MSCTF!CCompositeContextAdapter' inherits from 'MSCTF!CInputContext'.
   It is a wrapper for two 'MSCTF!CInputContext', where the second one
   is affected by 'ImmAssociateContext' WINAPI.
   I think that this is related to recently added emoji input panel.
3) 'MSCTF!CCompositeContextAdapter' is not removed on focus change
   'MSCTF!CInputContext::IsImmContext' contains some logic.
   'MSCTF!CCompositeContextAdapter::IsImmContext' always returns FALSE.
   This causes 'MSCTF!CInputContextAdapter::OnFocusChange' to skip
   branch that removes it from map when input focus leaves Edit.
   I think that this is a bug of 'IsImmContext' which returns FALSE.
   I imagine that it should return TRUE after 'ImmAssociateContext'.
4) Map is not cleaned when Edit has intermediate parent
   Map is also cleared when window or its parent is destroyed.
   But when Edit has two parents and the outer parent is destroyed,
   this code doesn't recognize the Edit and skips it.
   I think that this is a design flaw.
5) Map item is re-added when Edit is destroyed while having focus
   When Edit has focus while being destroyed, it's re-added to map in
   'SetFocus' handler. 'SetFocus' gets called as part of 'DestroyWindow'
   to pass input focus to some window.
   I think this that is a bug in Windows where it's not properly taken
   into account that Edit is being destroyed and should not be re-added.

'CCompositeContextAdapter' reference counting bug
-------------------------------------------------
1) 'MSCTF!CCompositeContextAdapter' uses 'MSCTF!CInputContext'
   without adding a reference.
   I think that this this is a bug in Windows.
2) Call to 'ImmAssociateContext(hwnd, 0)' fails to forget the dangling
   pointer. It will only forget old pointer if 'ImmAssociateContext' is
   used with some other non-NULL context.
   I think that this is not a problem on its own, but causes crash in
   combination with other bugs.
3) Call to 'ImmDestroyContext' destroys the 'MSCTF!CInputContext'
   This leaves a dangling pointer in 'MSCTF!CCompositeContextAdapter'.

Combined result
---------------
For many reasons, input contexts can stay in map forever.
'ImmDestroyContext' causes map to contain item with dangling pointer.
On certain actions entire map will be iterated.
If map contains items with dangling pointers, it will crash.

Change-Id: I78b7f75d86bb72d0746bc1455db61af9869ab839
Signed-off-by: Alexandr Miloslavskiy <alexandr.miloslavskiy@syntevo.com>

2 files changed, 144 insertions, 0 deletions

diff --git a/tests/org.eclipse.swt.tests.win32/.classpath b/tests/org.eclipse.swt.tests.win32/.classpath
index 2d54539e87..214e63ebf0 100644
--- a/tests/org.eclipse.swt.tests.win32/.classpath
+++ b/tests/org.eclipse.swt.tests.win32/.classpath
@@ -3,5 +3,6 @@
 	<classpathentry kind="con" path="org.eclipse.jdt.launching.JRE_CONTAINER/org.eclipse.jdt.internal.debug.ui.launcher.StandardVMType/JavaSE-1.8"/>
 	<classpathentry kind="con" path="org.eclipse.pde.core.requiredPlugins"/>
 	<classpathentry kind="src" path="JUnit Tests"/>
+	<classpathentry kind="src" path="ManualTests"/>
 	<classpathentry kind="output" path="bin"/>
 </classpath>
diff --git a/tests/org.eclipse.swt.tests.win32/ManualTests/org/eclipse/swt/tests/win32/snippets/Bug543747_JvmCrash_Msctf.java b/tests/org.eclipse.swt.tests.win32/ManualTests/org/eclipse/swt/tests/win32/snippets/Bug543747_JvmCrash_Msctf.java
new file mode 100644
index 0000000000..7bed16d8ea
--- /dev/null
+++ b/tests/org.eclipse.swt.tests.win32/ManualTests/org/eclipse/swt/tests/win32/snippets/Bug543747_JvmCrash_Msctf.java
@@ -0,0 +1,143 @@
+/*******************************************************************************

+ * Copyright (c) 2019 Syntevo and others. All rights reserved.

+ * The contents of this file are made available under the terms

+ * of the GNU Lesser General Public License (LGPL) Version 2.1 that

+ * accompanies this distribution (lgpl-v21.txt).  The LGPL is also

+ * available at http://www.gnu.org/licenses/lgpl.html.  If the version

+ * of the LGPL at http://www.gnu.org is different to the version of

+ * the LGPL accompanying this distribution and there is any conflict

+ * between the two license versions, the terms of the LGPL accompanying

+ * this distribution shall govern.

+ *

+ * Contributors:

+ *     Syntevo - initial API and implementation

+ *******************************************************************************/

+package org.eclipse.swt.tests.win32.snippets;

+

+import org.eclipse.swt.SWT;

+import org.eclipse.swt.layout.*;

+import org.eclipse.swt.widgets.*;

+

+public class Bug543747_JvmCrash_Msctf {

+	public static void reproduce526758(Shell parentShell) {

+		Shell tempShell = new Shell(parentShell);

+

+		// Create something to catch initial focus, so that

+		// text.setFocus() does something. This is only to

+		// show that .setFocus() is important.

+		new Button(tempShell, SWT.PUSH);

+

+		// This Text will cause crash.

+		Text text = new Text(tempShell, 0);

+

+		// Shell must be visible to prevent early return in .setFocus()

+		tempShell.setSize(10, 10);

+		tempShell.open();

+

+		// ImmAssociateContext() itself is lazy.

+		// .setFocus() causes it to start up.

+		text.setFocus();

+

+		// Destroying the shell triggers the bug.

+		tempShell.dispose();

+

+		// JVM still alive?

+		MessageBox msgbox = new MessageBox(parentShell);

+		msgbox.setMessage("Crash didn't reproduce");

+		msgbox.open();

+	}

+

+	public static void reproduce543747(Shell parentShell) {

+		Shell tempShell = new Shell(parentShell);

+

+		// Create something to catch initial focus, so that

+		// text.setFocus() does something. This is only to

+		// show that .setFocus() is important.

+		new Button(tempShell, SWT.PUSH);

+

+		// This Text will cause crash.

+		// Text needs to have an intermediate parent for this bug.

+		Composite composite = new Composite(tempShell, 0);

+		Text text = new Text(composite, 0);

+

+		// Shell must be visible to prevent early return in .setFocus()

+		tempShell.setSize(10, 10);

+		tempShell.open();

+

+		// ImmAssociateContext() itself is lazy.

+		// .setFocus() causes it to start up.

+		text.setFocus();

+

+		// Destroying the shell triggers the bug.

+		tempShell.dispose();

+

+		// Give additional instructions

+		MessageBox msgbox = new MessageBox(parentShell);

+		msgbox.setMessage(

+				"Now please do one of:\n" +

+				"a) Open Task Manager, go to Users, right-click your user, select Disconnect, log in again\n" +

+				"b) Connect Remote Desktop to this machine\n" +

+				"\n" +

+				"The snippet is expected to crash just after that."

+		);

+		msgbox.open();

+	}

+

+	public static void main (String [] args) {

+		Display display = new Display ();

+

+		Shell shell = new Shell (display);

+		RowLayout layout = new RowLayout(SWT.VERTICAL);

+		layout.marginHeight = 10;

+		layout.marginWidth = 10;

+		layout.spacing = 10;

+		shell.setLayout(layout);

+

+		final Text labelInfo = new Text(shell, SWT.READ_ONLY | SWT.MULTI);

+		labelInfo.setText(

+				"Both of these crashes are only seen on Win10 1809+\n" +

+				"\n" +

+				"To reproduce reliably, use Application Verifier:\n" +

+				"1) Install Application Verifier:\n" +

+				"a) Download Windows SDK:\n" +

+				"   https://go.microsoft.com/fwlink/p/?LinkID=2033908\n" +

+				"b) Install it, selecting Application Verifier. Other components are not required.\n" +

+				"\n" +

+				"2) Configure Application Verifier\n" +

+				"a) Run 'Application Verifier (X64)' from Start menu.\n" +

+				"b) Use File | Add application... to add java.exe\n" +

+				"c) IMPORTANT: On the right pane, make sure that only 'Basics/Heaps' is selected.\n" +

+				"   JVM always crashes with 'Basics/Exceptions' and 'Basics/Memory'\n" +

+				"d) Click 'Save'. You can close Application Verifier now\n" +

+				"   It will be active until you explicitly disable it.\n" +

+				"e) Restart application once.\n" +

+				"f) Note: applications run slower and consume more RAM under Application Verifier.\n" +

+				"\n" +

+				"3) Reproduce the problem\n" +

+				"\n" +

+				"4) Disable Application Verifier if you want\n" +

+				"a) Keep it enabled it if you're ready to tolerate the slowness, but find more bugs\n" +

+				"b) Go to Application Verifier again\n" +

+				"c) Delete java.exe from the list.\n" +

+				"d) Click Save.\n" +

+				"e) There's no need to uninstall Application Verifier, but you can do that if you like."

+		);

+

+		final Button button526758 = new Button(shell, SWT.PUSH);

+		button526758.setText("Reproduce crash 526758");

+		button526758.addListener(SWT.Selection, event -> {reproduce526758(shell);});

+

+		final Button button543747 = new Button(shell, SWT.PUSH);

+		button543747.setText("Reproduce crash 543747");

+		button543747.addListener(SWT.Selection, event -> {reproduce543747(shell);});

+

+		shell.pack();

+		shell.open();

+

+		while (!shell.isDisposed()) {

+			if (!display.readAndDispatch ()) display.sleep ();

+		}

+

+		display.dispose ();

+	}

+}