-
Bug
-
Resolution: Unresolved
-
P4
-
24
sun/awt/X11InputMethodBase defines a finalize() method which calls dispose();
I have had a look at this code and I think we can just remove it.
This code is complex, so some background is helpful.
The class hierarchy is
interface InputMethod
- defines abstract dispose() - this is an interface
^
|
abstract class InputMethodAdapter
^
|
abstract class X11InputMethodBase
- finalize()
- protected abstract disposeImpl()
- concrete definition of dispose() which calls disposeImpl() at most once.
^
|
abstract class X11InputMethod (there are aix and unix versions) of this
- overrides disposeImpl()
^
|
class XInputMethod
- over-rides disposeImpl()
So the disposeImpl methods do the actual clean up, and the public dispose() method used by clients
just makes sure it is only called once.
disposeImpl() resets many vars to null but this only matters for the case where the XInputMethod instance
is still live. Similarly the acquisition of the awtLock() doesn't matter in the non-live case, when being finalized
(or cleaned) there is no other thread using the vars that need to be locked out.
The AIX disposeImpl calls a method called clearComposedText() but that method does nothing unless
running on the EDT .. which the finalizer thread most certainly is not. So that won't change with a cleaner thread
and we can ignore this.
The only thing that matters in the finalize/clean case is the freeing of the native resource for the native
XInputContext. And that must be freed exactly once.
So a cleaner/disposer can just free that resource.
It appears to be the only resource that any of the disposeImpl() definitions free so we can handle this in
X11InputMethodBase which is where disposeXIC() is defined.
But a couple of things to be careful about.
The native method that is defined (compiled) is *platform-specific*.
But the real problem is that "this" - ie the InputMethod instance is used and
Can finalize ever be called ?
A GlobalRef is created for the instance,
globalRef = (*env)->NewGlobalRef(env, this);
pX11IMData->x11inputmethod = globalRef;
and it is only freed in disposeXIC() via destroyX11InputMethodData
(*env)->DeleteGlobalRef(env, pX11IMData->x11inputmethod);
[ actually there's some places it is re-created]
and I am struggling to understand this comment
* When XIC is created, a global reference is created for
* sun.awt.X11InputMethod object so that it could be used by the XIM callback
* functions. This could be a dangerous thing to do when the original
* X11InputMethod object is garbage collected and as a result,
* destroyX11InputMethodData is called to delete the global reference.
* If any XIM callback function still holds and uses the "already deleted"
* global reference, disaster is going to happen. So we have to maintain
* a list for these global references which is consulted first when the
* callback functions or any function tries to use "currentX11InputMethodObject"
* which always refers to the global reference try to use it.
How can it be GC'd if there's a global ref ? That 100% is a GC root.
Oh, it isn't being 'gc'd, it is being dispose()'d ? and the global ref is now no longer valid
whilst some native call back might still have it. Well that's another story.
But the point seems to me to be that unless dispose() is called you can't be finalized, so what's the point of finalize() ?
Also the global ref is stored in a native struct 'X11InputMethodData' as 'x11inputmethod'
which is referenced by the 'pData' field in the InputMethod instance.
This global ref seems to be needed for cases where call backs are provided to X11
and this at some point needs to do an up-call like this :-
onoffStatusWindow(...) {
parent = JNU_CallMethodByName(GetJNIEnv(), NULL, pX11IMData->x11inputmethod,
"getCurrentParentWindow",
"()J").j;
When focusGained (eg) is received on a component
sun.awt.im.InputContext.activateInputMethod(true) calls
X11InputMethod.activate() which calls
XInputMethod.createXIC() which calls
XInputMethod.createXICNative() which creates the X11InputMethodData - in the process calling NewGlobalRef
and stores it in sun/awt/X11InputMethodBase.pData
When
InputMethod.dispose() is called this calls
disposeImpl (see earlier) which calls
disposeXIC() (native method) which calls
destroyX11InputMethodData (native) which calls
freeX11InputMethodData which calls DeleteGlobalRef
I instrumented a build to monitor calls to finalize() and dispose() and used a test
that added a TextField to a window and called
InputContext ic = frame.getInputContext()
ic.selectInputMethod(Locale.JAPAN)
I could see the InputMethod created, the native context created.
Then after disposing the frame, the context was destroyed, the JNI ref releases, and finalize called.
If I commented out the call to DeleteGlobalRef() finalize was not called - because the instance is still alive.
So calling dispose() is a precondition to making it finalizable - if there is a native context.
And so calling dispose() from finalize is pointless.
I have had a look at this code and I think we can just remove it.
This code is complex, so some background is helpful.
The class hierarchy is
interface InputMethod
- defines abstract dispose() - this is an interface
^
|
abstract class InputMethodAdapter
^
|
abstract class X11InputMethodBase
- finalize()
- protected abstract disposeImpl()
- concrete definition of dispose() which calls disposeImpl() at most once.
^
|
abstract class X11InputMethod (there are aix and unix versions) of this
- overrides disposeImpl()
^
|
class XInputMethod
- over-rides disposeImpl()
So the disposeImpl methods do the actual clean up, and the public dispose() method used by clients
just makes sure it is only called once.
disposeImpl() resets many vars to null but this only matters for the case where the XInputMethod instance
is still live. Similarly the acquisition of the awtLock() doesn't matter in the non-live case, when being finalized
(or cleaned) there is no other thread using the vars that need to be locked out.
The AIX disposeImpl calls a method called clearComposedText() but that method does nothing unless
running on the EDT .. which the finalizer thread most certainly is not. So that won't change with a cleaner thread
and we can ignore this.
The only thing that matters in the finalize/clean case is the freeing of the native resource for the native
XInputContext. And that must be freed exactly once.
So a cleaner/disposer can just free that resource.
It appears to be the only resource that any of the disposeImpl() definitions free so we can handle this in
X11InputMethodBase which is where disposeXIC() is defined.
But a couple of things to be careful about.
The native method that is defined (compiled) is *platform-specific*.
But the real problem is that "this" - ie the InputMethod instance is used and
Can finalize ever be called ?
A GlobalRef is created for the instance,
globalRef = (*env)->NewGlobalRef(env, this);
pX11IMData->x11inputmethod = globalRef;
and it is only freed in disposeXIC() via destroyX11InputMethodData
(*env)->DeleteGlobalRef(env, pX11IMData->x11inputmethod);
[ actually there's some places it is re-created]
and I am struggling to understand this comment
* When XIC is created, a global reference is created for
* sun.awt.X11InputMethod object so that it could be used by the XIM callback
* functions. This could be a dangerous thing to do when the original
* X11InputMethod object is garbage collected and as a result,
* destroyX11InputMethodData is called to delete the global reference.
* If any XIM callback function still holds and uses the "already deleted"
* global reference, disaster is going to happen. So we have to maintain
* a list for these global references which is consulted first when the
* callback functions or any function tries to use "currentX11InputMethodObject"
* which always refers to the global reference try to use it.
How can it be GC'd if there's a global ref ? That 100% is a GC root.
Oh, it isn't being 'gc'd, it is being dispose()'d ? and the global ref is now no longer valid
whilst some native call back might still have it. Well that's another story.
But the point seems to me to be that unless dispose() is called you can't be finalized, so what's the point of finalize() ?
Also the global ref is stored in a native struct 'X11InputMethodData' as 'x11inputmethod'
which is referenced by the 'pData' field in the InputMethod instance.
This global ref seems to be needed for cases where call backs are provided to X11
and this at some point needs to do an up-call like this :-
onoffStatusWindow(...) {
parent = JNU_CallMethodByName(GetJNIEnv(), NULL, pX11IMData->x11inputmethod,
"getCurrentParentWindow",
"()J").j;
When focusGained (eg) is received on a component
sun.awt.im.InputContext.activateInputMethod(true) calls
X11InputMethod.activate() which calls
XInputMethod.createXIC() which calls
XInputMethod.createXICNative() which creates the X11InputMethodData - in the process calling NewGlobalRef
and stores it in sun/awt/X11InputMethodBase.pData
When
InputMethod.dispose() is called this calls
disposeImpl (see earlier) which calls
disposeXIC() (native method) which calls
destroyX11InputMethodData (native) which calls
freeX11InputMethodData which calls DeleteGlobalRef
I instrumented a build to monitor calls to finalize() and dispose() and used a test
that added a TextField to a window and called
InputContext ic = frame.getInputContext()
ic.selectInputMethod(Locale.JAPAN)
I could see the InputMethod created, the native context created.
Then after disposing the frame, the context was destroyed, the JNI ref releases, and finalize called.
If I commented out the call to DeleteGlobalRef() finalize was not called - because the instance is still alive.
So calling dispose() is a precondition to making it finalizable - if there is a native context.
And so calling dispose() from finalize is pointless.
- links to
-
Review(master) openjdk/jdk/26751