Multibit-Legacy / multibit-hd

Deprecated Bitcoin Wallet
https://multibit.org/blog/2017/07/26/multibit-shutdown.html
Other
172 stars 113 forks source link

Out of bounds exception in FileChooser #642

Closed jim618 closed 8 years ago

jim618 commented 9 years ago
{
"_index": "error-reports-201f433",
"_type": "log-entry",
"_id": "439",
"_score": null,
"_source": {
"@timestamp": "2015-06-30T23:20:25.975+02:00",
"level": "ERROR",
"thread_name": "AWT-EventQueue-0",
"logger_name": "org.multibit.hd.core.error_reporting.ExceptionHandler",
"message": "Uncaught exception",
"stack_trace": "java.lang.IndexOutOfBoundsException: Invalid index
 at javax.swing.DefaultRowSorter.convertRowIndexToModel(Unknown Source) ~[na:1.7.0_80]
 at sun.swing.FilePane$SortableListModel.getElementAt(Unknown Source) ~[na:1.7.0_80]
 at javax.swing.JList.getSelectedValue(Unknown Source) ~[na:1.7.0_80]
 at javax.swing.plaf.basic.BasicFileChooserUI$Handler.valueChanged(Unknown Source) ~[na:1.7.0_80]
 at javax.swing.JList.fireSelectionValueChanged(Unknown Source) ~[na:1.7.0_80]
 at javax.swing.JList$ListSelectionHandler.valueChanged(Unknown Source) ~[na:1.7.0_80]
 at javax.swing.DefaultListSelectionModel.fireValueChanged(Unknown Source) ~[na:1.7.0_80]
 at javax.swing.DefaultListSelectionModel.fireValueChanged(Unknown Source) ~[na:1.7.0_80]
 at javax.swing.DefaultListSelectionModel.setValueIsAdjusting(Unknown Source) ~[na:1.7.0_80]
 at javax.swing.JList.setValueIsAdjusting(Unknown Source) ~[na:1.7.0_80]
 at javax.swing.plaf.basic.BasicListUI$Handler.mouseReleased(Unknown Source) ~[na:1.7.0_80]
 at java.awt.AWTEventMulticaster.mouseReleased(Unknown Source) [na:1.7.0_80]
 at java.awt.AWTEventMulticaster.mouseReleased(Unknown Source) [na:1.7.0_80]
 at java.awt.Component.processMouseEvent(Unknown Source) [na:1.7.0_80]
 at javax.swing.JComponent.processMouseEvent(Unknown Source) [na:1.7.0_80]
 at java.awt.Component.processEvent(Unknown Source) [na:1.7.0_80]
 at java.awt.Container.processEvent(Unknown Source) [na:1.7.0_80]
 at java.awt.Component.dispatchEventImpl(Unknown Source) [na:1.7.0_80]
 at java.awt.Container.dispatchEventImpl(Unknown Source) [na:1.7.0_80]
 at java.awt.Component.dispatchEvent(Unknown Source) [na:1.7.0_80]
 at java.awt.LightweightDispatcher.retargetMouseEvent(Unknown Source) [na:1.7.0_80]
 at java.awt.LightweightDispatcher.processMouseEvent(Unknown Source) [na:1.7.0_80]
 at java.awt.LightweightDispatcher.dispatchEvent(Unknown Source) [na:1.7.0_80]
 at java.awt.Container.dispatchEventImpl(Unknown Source) [na:1.7.0_80]
 at java.awt.Window.dispatchEventImpl(Unknown Source) [na:1.7.0_80]
 at java.awt.Component.dispatchEvent(Unknown Source) [na:1.7.0_80]
 at java.awt.EventQueue.dispatchEventImpl(Unknown Source) [na:1.7.0_80]
 at java.awt.EventQueue.access$300(Unknown Source) [na:1.7.0_80]
 at java.awt.EventQueue$3.run(Unknown Source) [na:1.7.0_80]
 at java.awt.EventQueue$3.run(Unknown Source) [na:1.7.0_80]
 at java.security.AccessController.doPrivileged(Native Method) [na:1.7.0_80]
 at java.security.ProtectionDomain$1.doIntersectionPrivilege(Unknown Source) [na:1.7.0_80]
 at java.security.ProtectionDomain$1.doIntersectionPrivilege(Unknown Source) [na:1.7.0_80]
 at java.awt.EventQueue$4.run(Unknown Source) [na:1.7.0_80]
 at java.awt.EventQueue$4.run(Unknown Source) [na:1.7.0_80]
 at java.security.AccessController.doPrivileged(Native Method) [na:1.7.0_80]
 at java.security.ProtectionDomain$1.doIntersectionPrivilege(Unknown Source) [na:1.7.0_80]
 at java.awt.EventQueue.dispatchEvent(Unknown Source) [na:1.7.0_80]
 at java.awt.EventDispatchThread.pumpOneEventForFilters(Unknown Source) [na:1.7.0_80]
 at java.awt.EventDispatchThread.pumpEventsForFilter(Unknown Source) [na:1.7.0_80]
 at java.awt.EventDispatchThread.pumpEventsForFilter(Unknown Source) [na:1.7.0_80]
 at java.awt.WaitDispatchSupport$2.run(Unknown Source) [na:1.7.0_80]
 at java.awt.WaitDispatchSupport$4.run(Unknown Source) [na:1.7.0_80]
 at java.security.AccessController.doPrivileged(Native Method) [na:1.7.0_80]
 at java.awt.WaitDispatchSupport.enter(Unknown Source) [na:1.7.0_80]
 at java.awt.Dialog.show(Unknown Source) [na:1.7.0_80]
 at javax.swing.JFileChooser.showDialog(Unknown Source) [na:1.7.0_80]
 at javax.swing.JFileChooser.showOpenDialog(Unknown Source) [na:1.7.0_80]
 ...
gary-rowe commented 9 years ago

Possible sequence to reproduce (needs verifying):

  1. Create an empty directory c:\tmp\empty
  2. Start up MBHD, click on the file open button somewhere
  3. Navigate to c:\tmp
  4. Select empty (don't enter it, just single click on it).
  5. Click on the file open button again, this should drop you into c:\tmp\empty
  6. Click on the file open button again

Bit of a weird sequence that I've adapted from Java bug 6509187

jim618 commented 9 years ago

I could not reproduce this error using the sequence given in the previous post

jim618 commented 9 years ago

Looks like it is something to do with accessing the FileChooser on a non-Swing Thread: http://bugs.java.com/bugdatabase/view_bug.do?bug_id=6637181

jim618 commented 9 years ago

I have added a defensive SwingUtilities.invokeLater to the FileChooser picking code. Looking at the original code I don't see how this could be invoked by a non-Swing thread though as it is the result of a button click. Anyhow the change is pretty safe.

Awaiting review and closing.

gary-rowe commented 9 years ago

I'm not convinced that we've got the bottom of this. It's clear that we're accessing the FileChooser on the EDT. I'm inclined to think it's to do with setting the initial directory and that if that changes between invocations then we'll see an error if the file sizes are different.

While the defensive Swing wrapper is OK, I'd like to push this back into 0.1.3 for more attention.

jim618 commented 9 years ago

I have narrowed the scope of the JFileChooser so that it is created anew each time the file chooser button is pressed. Then an initial value is set if it is available in the model

I am hoping that by shortening the life cycle of the file chooser it will have less chance to get confused by file system changes (which is most likely what is happening)

Awaiting code review, testing and closing

jim618 commented 9 years ago

I have also tested this on a cold start successfully

gary-rowe commented 9 years ago

Code review looks good. Adding the invocation to a later item on the EDT queue should ensure that any earlier model operations within the file chooser are resolved before the next item is processed.

This is a probabilistic fix so let's put it out there and see if it recurs in 0.1.3. If so then we'll add further defensive code (such as trapping the IllegalArgumentException and performing a fixed number of repeat attempts before giving up).

Closing.

gary-rowe commented 8 years ago

Seen this reappear in log 1af2f7b under 0.1.4 so needs further attention. Further trawling through logs has it showing up quite a lot.

jim618 commented 8 years ago

Reading the two bug reports mentioned above 'setSelectedFile' appears problematic so I have rewritten the code to avoid invoking it. Now the constructor is used to set the initial file shown if it is available and is a directory.

This is a probable fix as the error is not reproducible.

Awaiting review and closing.

gary-rowe commented 8 years ago

Verified working. Closing (subject to ongoing probabilistic fix).