jPOS icon indicating copy to clipboard operation
jPOS copied to clipboard

fix: ChannelAdaptor disconnect lock now a new Object. Fixes #585

Open agustiza opened this issue 1 year ago • 4 comments

agustiza avatar Mar 11 '24 20:03 agustiza

Doig so e git archeology, it used to be a generic private Object but was changed into TRUE https://github.com/jpos/jPOS/commit/301e36a603203df8113e5eba01de97607cd456b7

The explanation in the commits mentions that the Object would complicate using a persistent space such as JESpace (not much explanation as to how.. serialization of a ChannelAdaptor? and the comment itself shows some doubts about the usefulness)

I agree that the boolean is troublesome… maybe a string object would be the best of both worlds?

barspi avatar Mar 12 '24 01:03 barspi

Good find!

Still I'm not sure the intention was to share the lock over a space (and it wouldn't really make sense if serialized over the wire).

Seems like the intention was to simply use a boolean for the space signaling and the lock was included under a overly broad replace.

agustiza avatar Mar 12 '24 09:03 agustiza

It seems that the disconnectLock variable, is only used in a synchronized block, wouldn't be better to fix this with a reentrant lock?

alcarraz avatar Mar 12 '24 12:03 alcarraz

Sure, they are pretty much equivalent in this case with the added benefit of being Loom proof should the sender or receiver become green threads in the future.

    protected void disconnect () {
        // do not synchronize on this as both Sender and Receiver can deadlock against a thread calling stop()
        disconnectLock.lock();
        try {
            SpaceUtil.wipe(sp, ready);
            channel.disconnect();
        } catch (Exception e) {
            getLog().warn("disconnect", e);
        } finally {
            disconnectLock.unlock();
        }
    }

agustiza avatar Mar 12 '24 13:03 agustiza