Instance Rename Exception
RobertBColton opened this issue · 2 comments
This does not appear to be a regression as it occurs all the way back to 1.8.10 and instance renaming did not exist in 1.8.6. I believe this to be an extension of the issue #159 that Josh reported, except with instances instead of the room properties which we supposedly fixed. Again, I can't test the versions in between these two right now because Java 9 doesn't like to load with the RTFEditorKit fixes I had in those versions.
Simply create a new object and place it in a new room. Save the room and reopen it. Attempt to rename the instance in the room and you will be met with the following exception.
Stack trace:
java.lang.IllegalStateException: Attempt to mutate in notification
at java.desktop/javax.swing.text.AbstractDocument.writeLock(AbstractDocument.java:1349)
at java.desktop/javax.swing.text.AbstractDocument.remove(AbstractDocument.java:590)
at org.lateralgm.ui.swing.propertylink.DocumentLink.setComponent(DocumentLink.java:37)
at org.lateralgm.ui.swing.propertylink.DocumentLink.setComponent(DocumentLink.java:1)
at org.lateralgm.util.PropertyLink.editComponent(PropertyLink.java:66)
at org.lateralgm.util.PropertyLink.reset(PropertyLink.java:54)
at org.lateralgm.util.PropertyLink.editProperty(PropertyLink.java:77)
at org.lateralgm.ui.swing.propertylink.DocumentLink.update(DocumentLink.java:70)
at org.lateralgm.ui.swing.propertylink.DocumentLink.insertUpdate(DocumentLink.java:58)
at java.desktop/javax.swing.text.AbstractDocument.fireInsertUpdate(AbstractDocument.java:203)
at java.desktop/javax.swing.text.AbstractDocument.handleInsertString(AbstractDocument.java:757)
at java.desktop/javax.swing.text.AbstractDocument.insertString(AbstractDocument.java:716)
at java.desktop/javax.swing.text.PlainDocument.insertString(PlainDocument.java:131)
at java.desktop/javax.swing.text.AbstractDocument.replace(AbstractDocument.java:675)
at java.desktop/javax.swing.text.JTextComponent.replaceSelection(JTextComponent.java:1339)
at java.desktop/javax.swing.text.DefaultEditorKit$DefaultKeyTypedAction.actionPerformed(DefaultEditorKit.java:884)
at java.desktop/javax.swing.SwingUtilities.notifyAction(SwingUtilities.java:1810)
at java.desktop/javax.swing.JComponent.processKeyBinding(JComponent.java:2900)
at java.desktop/javax.swing.JComponent.processKeyBindings(JComponent.java:2948)
at java.desktop/javax.swing.JComponent.processKeyEvent(JComponent.java:2862)
at java.desktop/java.awt.Component.processEvent(Component.java:6413)
at java.desktop/java.awt.Container.processEvent(Container.java:2263)
at java.desktop/java.awt.Component.dispatchEventImpl(Component.java:5012)
at java.desktop/java.awt.Container.dispatchEventImpl(Container.java:2321)
at java.desktop/java.awt.Component.dispatchEvent(Component.java:4844)
at java.desktop/java.awt.KeyboardFocusManager.redispatchEvent(KeyboardFocusManager.java:1950)
at java.desktop/java.awt.DefaultKeyboardFocusManager.dispatchKeyEvent(DefaultKeyboardFocusManager.java:870)
at java.desktop/java.awt.DefaultKeyboardFocusManager.preDispatchKeyEvent(DefaultKeyboardFocusManager.java:1139)
at java.desktop/java.awt.DefaultKeyboardFocusManager.typeAheadAssertions(DefaultKeyboardFocusManager.java:1009)
at java.desktop/java.awt.DefaultKeyboardFocusManager.dispatchEvent(DefaultKeyboardFocusManager.java:835)
at java.desktop/java.awt.Component.dispatchEventImpl(Component.java:4893)
at java.desktop/java.awt.Container.dispatchEventImpl(Container.java:2321)
at java.desktop/java.awt.Window.dispatchEventImpl(Window.java:2762)
at java.desktop/java.awt.Component.dispatchEvent(Component.java:4844)
at java.desktop/java.awt.EventQueue.dispatchEventImpl(EventQueue.java:772)
at java.desktop/java.awt.EventQueue$4.run(EventQueue.java:721)
at java.desktop/java.awt.EventQueue$4.run(EventQueue.java:715)
at java.base/java.security.AccessController.doPrivileged(AccessController.java:391)
at java.base/java.security.ProtectionDomain$JavaSecurityAccessImpl.doIntersectionPrivilege(ProtectionDomain.java:85)
at java.base/java.security.ProtectionDomain$JavaSecurityAccessImpl.doIntersectionPrivilege(ProtectionDomain.java:95)
at java.desktop/java.awt.EventQueue$5.run(EventQueue.java:745)
at java.desktop/java.awt.EventQueue$5.run(EventQueue.java:743)
at java.base/java.security.AccessController.doPrivileged(AccessController.java:391)
at java.base/java.security.ProtectionDomain$JavaSecurityAccessImpl.doIntersectionPrivilege(ProtectionDomain.java:85)
at java.desktop/java.awt.EventQueue.dispatchEvent(EventQueue.java:742)
at java.desktop/java.awt.EventDispatchThread.pumpOneEventForFilters(EventDispatchThread.java:203)
at java.desktop/java.awt.EventDispatchThread.pumpEventsForFilter(EventDispatchThread.java:124)
at java.desktop/java.awt.EventDispatchThread.pumpEventsForHierarchy(EventDispatchThread.java:113)
at java.desktop/java.awt.EventDispatchThread.pumpEvents(EventDispatchThread.java:109)
at java.desktop/java.awt.EventDispatchThread.pumpEvents(EventDispatchThread.java:101)
at java.desktop/java.awt.EventDispatchThread.run(EventDispatchThread.java:90)
So the reason this happens and the reason the original issue happened is not because there's a memory leak. UpdateSource correctly implements the observer pattern in a way that does solve the lapsed listener problem. The issue is with PropertyLink itself and its subclasses relying on the weak referencing to do their cleanup after resource frames are closed.
This is wrong because garbage collection may never run during the lifetime of an application, which is the case with the non-garbage collecting reference JVM implementation. If you are relying on finalization to do cleanup of non-native resources, you're doing something wrong. This means my original fix for the default property link factory to clear its links when a resource frame is disposed was in fact correct, and it needs to be extended to the other secondary property link factories that still have issues like the above.
There are a couple of ways this can be resolved:
- Have the relevant frames override dispose, call the super dispose, and manually clear their secondary link factories.
- Overload PropertyLinkFactory to handle multiple maps, eliminating the need for secondary link factories.
- Track link factory instances with a factory or in their constructor in a way that base resource frame can clear related factories.
Closing as resolved by 49115dd which adds a concept of ownership to the factories.