Skip to content

8365180: Remove sun.awt.windows.WInputMethod.finalize() #26706

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 2 commits into from
Closed

8365180: Remove sun.awt.windows.WInputMethod.finalize() #26706

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
fc422aa
8365180
prrace Aug 8, 2025
3c8c9cb
8365180
prrace Aug 19, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 24 additions & 14 deletions src/java.desktop/windows/classes/sun/awt/windows/WInputMethod.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,8 @@
import sun.awt.AWTAccessor;
import sun.awt.AWTAccessor.ComponentAccessor;
import sun.awt.im.InputMethodAdapter;
import sun.java2d.Disposer;
import sun.java2d.DisposerRecord;

final class WInputMethod extends InputMethodAdapter
{
Expand Down Expand Up @@ -124,6 +126,8 @@ final class WInputMethod extends InputMethodAdapter
public WInputMethod()
{
context = createNativeContext();
disposerRecord = new ContextDisposerRecord(context);
Disposer.addRecord(this, disposerRecord);
cmode = getConversionStatus(context);
open = getOpenStatus(context);
currentLocale = getNativeLocale();
Expand All @@ -132,16 +136,23 @@ public WInputMethod()
}
}

@Override
@SuppressWarnings("removal")
protected void finalize() throws Throwable
{
// Release the resources used by the native input context.
if (context!=0) {
destroyNativeContext(context);
context=0;
private final ContextDisposerRecord disposerRecord;

private static final class ContextDisposerRecord implements DisposerRecord {

private final int context;
private volatile boolean disposed;

ContextDisposerRecord(int c) {
context = c;
}

public synchronized void dispose() {
if (!disposed) {
destroyNativeContext(context);
}
disposed = true;
}
super.finalize();
}

@Override
Expand All @@ -151,9 +162,7 @@ public synchronized void setInputMethodContext(InputMethodContext context) {

@Override
public void dispose() {
// Due to a memory management problem in Windows 98, we should retain
// the native input context until this object is finalized. So do
// nothing here.
disposerRecord.dispose();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The native dispose will call ImmDestroyContext which is specified as:

However, before calling ImmDestroyContext, the application must remove the input context from any association with windows in the thread by using the ImmAssociateContext function.

I did not check the code, but maybe we are calling ImmAssociateContext in the wrong moment and have to postpone this "dispose" until all the object is really unused?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems to be done by a native method WInputMethod.disableNativeIME(..) - which sets the IME for the window to NULL. The native method is called from various places in WInputMethod.
I can't see that dispose or finalize would do anything to ensure that is called, or introduce any timing issues either way. So if there's a problem it is un-related.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't see that dispose or finalize would do anything to ensure that is called, or introduce any timing issues either way. So if there's a problem it is un-related.

Maybe disableNativeIME is called when the top level Window is disposed, and after that there are no more references to WInputMethod? In that case, it is safe to call it from finalize(), but it might not be as safe to call it from dispose(), which mighe be called before "WInputMethod.disableNativeIME"?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe disableNativeIME is called when the top level Window is disposed,

No it is not.

Nothing calls disableNativeIME when the window is disposed. I tried this with the disposer removed and finalize restored just to be sure.

Also it needs to be called 'early' but not 'too early' during WIndow.dispose()
If the input method is active when you try to dispose it via APIs like disableNativeIM() then an exception is thrown.

If you call it too late - after removeNotify() then it will not be sent because the peer is now nulled out.

src/java.desktop/windows/native/libawt/windows/awt_Toolkit.cpp
@@ -1082,6 +1082,7 @@ LRESULT CALLBACK AwtToolkit::WndProc(HWND hWnd, UINT message,
AwtComponent* comp = (AwtComponent*)JNI_GET_PDATA(peer);
if (comp != NULL)
{
comp->SetInputMethod(self, useNativeCompWindow);
comp->ImmAssociateContext((HIMC)((intptr_t)context));
}

Since we never call it, then this migration to disposer changes nothing.
If we do call it I think it should be called in WInputMethod.removeNotify() which will clearly be before dispose().
I will add that and we'll be better off then we were before this change.

I also found that if the window has focus when it is dispose()'d that it will receive a focus lost event which ironically creates a new WInputMethod because Window.dispose() had nulled out the existing one

Either don't null out the disposed one, or do something else to avoid re-creation, or decide that it is harmless .
For now I will leave that as it seems to be harmless, pre-existing and un-related to this change, although a little wasteful. It also would be more than a windows-only change.

}

/**
Expand Down Expand Up @@ -448,6 +457,7 @@ public void hideWindows() {
@Override
public void removeNotify() {
endCompositionNative(context, DISCARD_INPUT);
disableNativeIME(awtFocussedComponentPeer);
awtFocussedComponent = null;
awtFocussedComponentPeer = null;
}
Expand Down Expand Up @@ -658,8 +668,8 @@ private WComponentPeer getNearestNativePeer(Component comp)

}

private native int createNativeContext();
private native void destroyNativeContext(int context);
private static native int createNativeContext();
private static native void destroyNativeContext(int context);
private native void enableNativeIME(WComponentPeer peer, int context, boolean useNativeCompWindow);
private native void disableNativeIME(WComponentPeer peer);
private native void handleNativeIMEEvent(WComponentPeer peer, AWTEvent e);
Expand Down
4 changes: 2 additions & 2 deletions src/java.desktop/windows/native/libawt/windows/awt_InputMethod.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ extern BOOL g_bUserHasChangedInputLang;
* Signature: ()I
*/
JNIEXPORT jint JNICALL
Java_sun_awt_windows_WInputMethod_createNativeContext(JNIEnv *env, jobject self)
Java_sun_awt_windows_WInputMethod_createNativeContext(JNIEnv *env, jclass cls)
{
TRY;

Expand All @@ -69,7 +69,7 @@ Java_sun_awt_windows_WInputMethod_createNativeContext(JNIEnv *env, jobject self)
* Signature: (I)V
*/
JNIEXPORT void JNICALL
Java_sun_awt_windows_WInputMethod_destroyNativeContext(JNIEnv *env, jobject self, jint context)
Java_sun_awt_windows_WInputMethod_destroyNativeContext(JNIEnv *env, jclass cls, jint context)
{
TRY_NO_VERIFY;

Expand Down