IndexOutOfBoundsException updating a JTable - multi-threaded race condition?

[mt-inside]

Steps

• Start FS2 with one share with some files in it, running as forced indexnode.

Actual

• This isn't always reproducible, making me think it's a race.
• See log with backtraces below.
• Looks like one thread is doing HTTP work (an upload) and marshals a call onto another thread to update the Uploads table in the GUI. BY the time it gets there the table is in a state that makes its operation invalid.

2012.08.06 23:16:07 INFO: Now exporting shares on port 41234
2012.08.06 23:16:07 INFO: Checking for updates...
2012.08.06 23:16:07 INFO: Refresh complete (share Default download directory on magrathea)
2012.08.06 23:16:07 INFO: The indexnode at http://127.0.0.1:1337 is now known as 'magrathea'
2012.08.06 23:16:08 WARNING: Couldn't remove from uploads table: java.lang.reflect.InvocationTargetException
java.lang.reflect.InvocationTargetException
at java.awt.EventQueue.invokeAndWait(EventQueue.java:1045)
at javax.swing.SwingUtilities.invokeAndWait(SwingUtilities.java:1347)
at client.gui.Utilities.dispatch(Utilities.java:248)
at client.gui.Utilities.dispatch(Utilities.java:272)
at client.shareserver.ShareServer$UploadsTableModel.transferEnded(ShareServer.java:185)
at client.shareserver.ShareServer$HttpEventsImpl.transferEnded(ShareServer.java:271)
at common.HttpUtil$HttpTransferInfo.transferComplete(HttpUtil.java:293)
at common.HttpFileHandler.handle(HttpFileHandler.java:106)
at common.httpserver.Filter$Chain.doFilter(Filter.java:34)
at client.indexnode.IndexNodeOnlyFilter.doFilter(IndexNodeOnlyFilter.java:46)
at common.httpserver.Filter$Chain.doFilter(Filter.java:36)
at common.FS2Filter.doFilter(FS2Filter.java:92)
at common.httpserver.Filter$Chain.doFilter(Filter.java:36)
at common.httpserver.impl.ExchangeImpl.(ExchangeImpl.java:123)
at common.httpserver.impl.ServerImpl$1$1.run(ServerImpl.java:128)
at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:471)
at java.util.concurrent.FutureTask$Sync.innerRun(FutureTask.java:334)
at java.util.concurrent.FutureTask.run(FutureTask.java:166)
at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1110)
at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:603)
at java.lang.Thread.run(Thread.java:679)
Caused by: java.lang.IndexOutOfBoundsException: Invalid range
at javax.swing.DefaultRowSorter.rowsDeleted(DefaultRowSorter.java:880)
at javax.swing.JTable.notifySorter(JTable.java:4278)
at javax.swing.JTable.sortedTableChanged(JTable.java:4122)
at javax.swing.JTable.tableChanged(JTable.java:4399)
at client.shareserver.ShareServer$UploadsTableModel$2.run(ShareServer.java:190)
at client.gui.Utilities$1.run(Utilities.java:241)
at java.awt.event.InvocationEvent.dispatch(InvocationEvent.java:216)
at java.awt.EventQueue.dispatchEventImpl(EventQueue.java:647)
at java.awt.EventQueue.access$000(EventQueue.java:96)
at java.awt.EventQueue$1.run(EventQueue.java:608)
at java.awt.EventQueue$1.run(EventQueue.java:606)
at java.security.AccessController.doPrivileged(Native Method)
at java.security.AccessControlContext$1.doIntersectionPrivilege(AccessControlContext.java:105)
at java.awt.EventQueue.dispatchEvent(EventQueue.java:617)
at java.awt.EventDispatchThread.pumpOneEventForFilters(EventDispatchThread.java:275)
at java.awt.EventDispatchThread.pumpEventsForFilter(EventDispatchThread.java:200)
at java.awt.EventDispatchThread.pumpEventsForHierarchy(EventDispatchThread.java:190)
at java.awt.EventDispatchThread.pumpEvents(EventDispatchThread.java:185)
at java.awt.EventDispatchThread.pumpEvents(EventDispatchThread.java:177)
at java.awt.EventDispatchThread.run(EventDispatchThread.java:138)

[conicalflask]

Ok, so FS2's dynamic gui elements are riddled with race conditions and mistakes, but they're fiercely complicated, even in theory. Most of them are caught so they dont do any damage and they self correct as time goes on.

I have a feeling if you say this one is close to program startup that the indexnode is requesting the file lists (an upload from the client) before the gui has initialised. The file transfers then finish after the gui has initialised so the gui receives the "transfer complete" event, but there's nothing to update in the table of transfers.

I'm not sure what the elegant fix is for this. I'll ponder for a while.

[conicalflask]

On the plus though, you can see that at least this was anticipated and caught :)
"2012.08.06 23:16:08 WARNING: Couldn't remove from uploads table: "

[mt-inside]

No worries; I'm just raising all the bugs I can find. And NullRef == bug :)

[conicalflask]

Only if it's uncaught :D

Stacktraces are not necessarily bugs.

On Mon, Aug 6, 2012 at 11:53 PM, Matt Turner notifications@github.comwrote:

No worries; I'm just raising all the bugs I can find. And NullRef == bug :)

—
Reply to this email directly or view it on GitHubhttps://github.com//issues/5#issuecomment-7538967.

Hah, totally decided to take a look at this and then immediately decided it wasn't worth it! Swing is a bit of a nightmare.

[conicalflask]

:)
In this case I'm pretty sure it's the race where the indexnode requests a filelist before the gui is initialised. The potential fixes are:

1. Mark transfers that start before the gui is ready as non-gui and don't try to remove them from the table.
2. Prevent notifying indexnodes until the gui is ready.
3. Silently ignore the exceptions in this case.
4. Do nothing because it's benign.

I'm inclined for 1, 3, or 4. Four is easy. I'm doing it right now :D

I'm not sure what the current architecture of the GUI is with relation to the underlying components, but I think it would be sensible to basically have an FS2 service running, and the GUI slapped on top of it on a strict set of API calls. To simply things, the GUI should only (effectively) poll the service and update itself based on the returns. Getting some clean separation in the architecture should keep the performance high as well as isolating the gui behaviour, meaning much more simple code and removing all of those nasty potential race conditions.

[conicalflask]

That is essentially how it's architected, but there are also events which allow the gui to update only when needed.

Each of FS2's subsystems are reasonably isolated, for example the servers and indexers don't care if the gui is present or not and will issue events to anything listening.

The gui bugs stem primarily from swing's table and tree models being so hard to drive and so my code is error prone.

This artefact, however, is not one of those bugs and does not stem from a lack of separation between gui and service but in fact because of it.

The event dispatch in this case doesn't care about whether the GUI is able to handle the event. It's just reporting facts.

The uploads table was coded with the assumption that if a transfer ended, it must have started and therefore must be in the table.

Next time we have a beer I need to verbally fight you for that comment though. It took me a long time to draft and redraft this final paragraph.