Dirk Stöcker wrote: > On Sat, 3 Oct 2009, Ray Foulkes wrote: > ...etc. > > The reason is easy -> Make the data access thread safe. I think there are > "sync" keywords in Java to do so, but I do not really understand Java > multithreading completely (I'm more a C/C++ guy :-) >
Hi Dirk, thanks for responding. I wasn't quite ready to propose what to change to fix the problem, but here goes with my current thoughts... I read "JTT"'s observation attached to the (closed)#3238 ticket: "livegps is modifying the gpx track at the same time as EDT is trying to get length of the track to show tooltip text. The exception can be fixed either by making GpxData thread safe (using ConcurrentLinkedQueue for GpxTrack.trackSegs instead of LinkedList will fix this concrete issue) or by acessing GpxData only in worker thread." I agree that it is true that the exception could be avoided by making gpxdata thread safe as JTT suggested using ConcurrentLinkedQueue. BUT I am reluctant to go down that route for the following reasons: 1. It is dangerous to allow unsynchronized access to any data structure (such as "Layer" and its subclasses) from more than one thread. The main part of JOSM accesses "layers"(and it subclasses) from the event dispatch thread of swing. JOSM was "lucky" in the sense that the linked queue being used is concurrency aware (if not concurrency protected) and raised the concurrent modification exception. There may be other accesses simply giving/providing bad data i.e. any access of a non-atomic data item might give some part of an old version and some part of the next to a reader. Even if you could be sure of protecting all the accesses of the current program, someone innocently might add something else which is not synchronized. Unless it can be absolutely assured that the ONLY data changed from any thread other than the swing event dispatcher thread is gpxdata, then simply protecting gpxdata can only be regarded as patching over the problem. Accessing gpxdata only via another thread is likely to incur rather a high overhead for (for example) paint. 2. There is another problem in livegps as follows: The propertychangelistener of LiveGpsDialog accepts "gpsdata" change events. It then calls javax.swing.JComponent.setBackground (and a few other swing components). Since I am fairly sure that "gpsdata" bean events are delivered synchronously from the livegpsacquirer thread, it is a non-permitted use of Swing calls see: http://java.sun.com/javase/6/docs/api/javax/swing/package-summary.html#threading which explains that the only thread permitted to invoke swing primitives is the swing event dispatch thread. Calling swing components from another thread is undetected by Swing but is banned for a very good reason; nothing is synchronized in Swing. The developers of swing wrote a long explanation as to why this was. It would be possible to program a method of feeding events back into the event system of Swing, all of which are executed on the Swing dispatching thread (unlike the beans events which are synchronous with the thread of the declarer). Then of course, the event handler on the swing thread would have to use synchronized access to anything modified in the livegpsacquirer thread but could legally call swing components. On the other hand there is a recommendation from the Swing developers on how to carry out asynchronous background processing and a package to support it. Essentially it supports feeding a queue of objects back to a method called on the swing event dispatcher thread where swing operations can be invoked. see http://java.sun.com/docs/books/tutorial/uiswing/concurrency/worker.html Sorry this is a bit long but... My initial thought was to try to avoid having to synchronize "layer" (or its various components) and its sub classes. Doing that would be safe but may incur too great an overhead cost for the rest of JOSM. It might be better if livegpsacquirer returned new objects to a handler running on the swing event dispatcher thread as supported by the Swingworker class. Thus there would be no concurrent access to "layer" and its subclasses. BUT, I have not yet looked at what other threads need access to "layer" (and subclasses) in other packages. Hence I am not at this moment sure of my ground. There maybe no alternative to synchronizing various parts of layer and its subclasses, but I think that might be lots of work. There is also some bad news/good news regarding Swingworker; JOSM has only a reliance on SE5, Swingworker is only integrated with SE6. I have downloaded the source of the Swingworker package and it looks as if it will compile and run under SE5 - on the other hand it is Sun copyright, and so delivering it with the package may well be illegal. sigh. As you are aware, at the moment all this is based on code reading since I cannot yet build and debug JOSM + plugins. In summary, I am nervous about simply putting in a patch to stop this exception being raised. It would certainly solve my immediate problem using JOSM+livegps+surveyor, but I don't think that it would be doing the JOSM project a great favour for the future. Cheers, and assuring you of my best intentions, Ray _______________________________________________ josm-dev mailing list josm-dev@openstreetmap.org http://lists.openstreetmap.org/listinfo/josm-dev
