On 2012-01-15 07:34, Brett Henderson wrote:
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.
Does it make sense to change the Releasable API definition to say that release can only be called once? Or might that break multiple-sink stages like merge?
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.
...
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 :-)
Looking at the code a bit more I think it might only need minor changes. I had thought that release() and outputRelease() changed the state to equal that of a freshly constructed DataPostbox (to support reuse?), but that's not true: inputExit and outputExit are false after construction but true after release(). So simply checking inputExit at the beginning of release() should take care of it. See the attached (untested) patch.
An alternative is to increase the burden on the users of DataPostbox. They could be required to either ensure that the output thread restarts or avoid calling DataPostbox.release() once the output thread exits. For example, EntityBuffer.run() could set an 'outputThreadIsRunning' boolean at the top and clear it at the end. Then EntityBuffer.release() would be modified to only call buffer.release() if outputThreadIsRunning is true.
-Richard
>From 86c93d88e011ab76b79737c6185bd138f2eced76 Mon Sep 17 00:00:00 2001 From: Richard Hansen <[email protected]> Date: Sun, 15 Jan 2012 15:13:01 -0500 Subject: [PATCH] Do nothing in DataPostbox.release() if already released The Releasable interface specification says that release() may be called multiple times. Before, if DataPostbox.release() was called a second time, it would block forever waiting for the output thread (which would no longer exist) to release. Now subsequent calls to release() are a no-op, preventing the lockup. --- .../osmosis/core/store/DataPostbox.java | 5 +++++ 1 files changed, 5 insertions(+), 0 deletions(-) diff --git a/core/src/main/java/org/openstreetmap/osmosis/core/store/DataPostbox.java b/core/src/main/java/org/openstreetmap/osmosis/core/store/DataPostbox.java index 372de7f..415ee40 100644 --- a/core/src/main/java/org/openstreetmap/osmosis/core/store/DataPostbox.java +++ b/core/src/main/java/org/openstreetmap/osmosis/core/store/DataPostbox.java @@ -335,6 +335,11 @@ public class DataPostbox<T> implements Initializable { lock.lock(); try { + // support release() being called multiple + // times (do nothing if already released) + if (inputExit) + return; + // If release is being called without having completed successfully, // it is an error condition. if (!inputComplete) { -- 1.7.8.2
_______________________________________________ osmosis-dev mailing list [email protected] http://lists.openstreetmap.org/listinfo/osmosis-dev
