On 15 January 2012 14:09, Richard Hansen <rhan...@bbn.com> wrote: > Hi all, > > I believe I have found a bug in DataPostbox.release(). (I'm using git > master, currently 43089ed.) > > According to the javadoc comments, Releaseable.release() can be called > multiple times. However, DataPostbox.release() can only be called once. > If it is called a second time, it blocks forever waiting for the output > thread (which no longer exists) to release. >
Good find, thanks for the report. You're correct in saying that Releasable classes can be released multiple times, however that is not typically how they're used in the Osmosis pipeline. DataPostbox is a bit tricky because it is the main synchronisation point between multiple threads. Two threads communicating via a DataPostbox perform handshakes during the initialize, complete, and release methods (the process methods run mostly independently). It is the release method where each threads tells the other thread that they are about to exit. DataPostbox does technically support multiple calls to release, but it would require each thread to restart after the release calls and this isn't supported (or even desirable) by the Osmosis pipeline. > > I noticed this because the pbf reader can call release() twice, triggering > the lockup. To recreate, you can run the following: > > osmosis --read-pbf /dev/null --buffer --write-null > > The two calls to DataPostbox.release() have the following stack traces: > > java.lang.Exception: Stack trace > at java.lang.Thread.dumpStack(**Thread.java:1266) > at org.openstreetmap.osmosis.**core.store.DataPostbox.** > release(DataPostbox.java:335) > at org.openstreetmap.osmosis.**core.buffer.v0_6.EntityBuffer.** > release(EntityBuffer.java:64) > at crosby.binary.osmosis.**OsmosisBinaryParser.complete(** > OsmosisBinaryParser.java:36) > at crosby.binary.file.**BlockInputStream.process(** > BlockInputStream.java:37) > at crosby.binary.osmosis.**OsmosisReader.run(** > OsmosisReader.java:45) > at java.lang.Thread.run(Thread.**java:679) > java.lang.Exception: Stack trace > at java.lang.Thread.dumpStack(**Thread.java:1266) > at org.openstreetmap.osmosis.**core.store.DataPostbox.** > release(DataPostbox.java:335) > at org.openstreetmap.osmosis.**core.buffer.v0_6.EntityBuffer.** > release(EntityBuffer.java:64) > at crosby.binary.osmosis.**OsmosisReader.run(** > OsmosisReader.java:50) > at java.lang.Thread.run(Thread.**java:679) > > Deleting line 36 of > pbf/src/main/java/crosby/**binary/osmosis/**OsmosisBinaryParser.java > will cause DataPostbox.release() to be called only once, but that just > avoids the bug. > I've removed line 36 as you've suggested because it shouldn't be there. Release should only be called in a finally block that gets called regardless of success or failure, the complete method only gets called on success. I think it was me that added the other release in OsmosisReader recently when I added initialize, but apparently I didn't make a corresponding change in OsmosisBinaryParser. Please let me know if you see any further issues. > > I believe a proper fix to DataPostbox.release() will require a minor > redesign of the DataPostbox lifetime management code. I'm not familiar > enough with the code to feel comfortable making such a change. I can take > a crack at it, but I'm hoping an expert has the time to take a look. > What changes would you suggest? I believe it's working as I originally intended, but that's not to say there isn't a better way :-) Brett
_______________________________________________ osmosis-dev mailing list osmosis-dev@openstreetmap.org http://lists.openstreetmap.org/listinfo/osmosis-dev
