re-binding sequentially, potentially fixes #40

properties/src/main/kotlin/net/aquadc/properties/internal/-Listeners.kt

@@ -399,16 +399,25 @@ abstract class `-Listeners`<out T, in D, LISTENER : Any, UPDATE> : AtomicReferen
     }
 
     private fun concChangeObservedStateTo(obsState: Boolean) {
-        val firsState = concState().getUndUpdate { prev ->
+        var prev: ConcListeners<LISTENER, UPDATE>
+        var next: ConcListeners<LISTENER, UPDATE>
+        do {
+            prev = concState().get()
+            while (prev.transitionLocked) {
+                // if we can't transition for external reasons (e. g. in ConcMutableProperty), just wait until this ends
+                Thread.yield()
+                prev = concState().get()
+            }
+
             if (prev.nextObservedState == obsState) {
                 // do nothing if we're either transitioning to current state or already there
                 return
             }
 
-            prev.startTransition()
-        }
+            next = prev.startTransition()
+        } while (!concState().compareAndSet(prev, next))
 
-        if (firsState.transitioningObservedState) {
+        if (prev.transitioningObservedState) {
             // do nothing if this method is already on the stack somewhere
             return
         }
@@ -433,7 +442,30 @@ abstract class `-Listeners`<out T, in D, LISTENER : Any, UPDATE> : AtomicReferen
         }
     }
 
-    /*...not overridden in [ConcMutableDiffProperty], because it is not mapped and cannot be bound. */
+    /* intentionally does not contain try..catch */
+    internal inline fun withLockedTransition(block: () -> Unit) {
+        while (!tryLockTransition()) Thread.yield()
+        block()
+        unlockTransition()
+    }
+
+    internal fun tryLockTransition(): Boolean {
+        val prev = concState().get()
+        if (prev.transitionLocked || prev.transitioningObservedState) {
+            return false
+        }
+
+        return concState().compareAndSet(prev, prev.flippedTransitionLock())
+    }
+
+    internal fun unlockTransition() {
+        val old = concState().getAndUpdate(ConcListeners<LISTENER, UPDATE>::flippedTransitionLock)
+        check(old.transitionLocked)
+    }
+
+    /*...not overridden in [ConcMutableDiffProperty], because it is not mapped and cannot be bound.
+     * This callback can be called only once at a time (i. e. under mutex) for a single property,
+     * and with transitionLocked=false */
     internal open fun observedStateChanged(observed: Boolean) {}
 
     @Suppress("NOTHING_TO_INLINE", "UNCHECKED_CAST")

properties/src/main/kotlin/net/aquadc/properties/internal/ConcListeners.kt

@@ -12,11 +12,12 @@ internal class ConcListeners<out L : Any, out T>(
         @JvmField val listeners: Array<out L?>,
         @JvmField val pendingValues: Array<out T>,
         @JvmField val transitioningObservedState: Boolean,
-        @JvmField val nextObservedState: Boolean
+        @JvmField val nextObservedState: Boolean,
+        @JvmField val transitionLocked: Boolean
 ) {
 
     fun withListener(newListener: @UnsafeVariance L): ConcListeners<L, T> =
-            ConcListeners(listeners.with(newListener) as Array<L?>, pendingValues, transitioningObservedState, nextObservedState)
+            ConcListeners(listeners.with(newListener) as Array<L?>, pendingValues, transitioningObservedState, nextObservedState, transitionLocked)
 
     fun withoutListenerAt(idx: Int): ConcListeners<L, T> {
         val newListeners = when {
@@ -36,11 +37,11 @@ internal class ConcListeners<out L : Any, out T>(
         return if (pendingValues.isEmpty() && newListeners.isEmpty() && !transitioningObservedState && !nextObservedState)
             NoListeners
         else
-            ConcListeners(newListeners, pendingValues, transitioningObservedState, nextObservedState)
+            ConcListeners(newListeners, pendingValues, transitioningObservedState, nextObservedState, transitionLocked)
     }
 
     fun withNextValue(newValue: @UnsafeVariance T): ConcListeners<L, T> =
-            ConcListeners(listeners, pendingValues.with(newValue) as Array<out T>, transitioningObservedState, nextObservedState)
+            ConcListeners(listeners, pendingValues.with(newValue) as Array<out T>, transitioningObservedState, nextObservedState, transitionLocked)
 
     fun next(): ConcListeners<L, T> {
         val listeners = if (this.pendingValues.size == 1) {
@@ -51,17 +52,20 @@ internal class ConcListeners<out L : Any, out T>(
         }
 
         // remove value at 0, that listeners were just notified about
-        return ConcListeners(listeners, pendingValues.copyOfWithout(0, EmptyArray) as Array<out T>, transitioningObservedState, nextObservedState)
+        return ConcListeners(listeners, pendingValues.copyOfWithout(0, EmptyArray) as Array<out T>,
+                transitioningObservedState, nextObservedState, transitionLocked)
     }
 
-    fun startTransition(): ConcListeners<L, T> {
-        return ConcListeners(listeners, pendingValues, true, !nextObservedState)
-    }
+    fun startTransition(): ConcListeners<L, T> =
+            ConcListeners(listeners, pendingValues, true, !nextObservedState, transitionLocked)
 
     fun continueTransition(appliedState: Boolean): ConcListeners<L, T> {
         check(transitioningObservedState)
         val done = appliedState == nextObservedState
-        return ConcListeners(listeners, pendingValues, !done, nextObservedState)
+        return ConcListeners(listeners, pendingValues, !done, nextObservedState, transitionLocked)
     }
 
+    fun flippedTransitionLock(): ConcListeners<L, T> =
+            ConcListeners(listeners, pendingValues, transitioningObservedState, nextObservedState, !transitionLocked)
+
 }

properties/src/main/kotlin/net/aquadc/properties/internal/ConcMutable-.kt

@@ -16,47 +16,45 @@ internal class `ConcMutable-`<T>(
         true, value
 ), MutableProperty<T>, ChangeListener<T> {
 
-    override var value: T // field: T | Binding<T> | rebinding()
+    override var value: T // field: T | Binding<T>
         get() {
-            var value: Any?
-            do { value = valueUpdater().get(this) } while (value === rebinding())
-            return if (value is Binding<*>) (value as Binding<T>).original.value else value as T
+            val value = valueUpdater().get(this)
+            return if (value is Binding<*>) {
+                value as Binding<T>
+                val retValue: T
+                if (!isBeingObserved()) {
+                    // we're not observed, so stale value may be remembered — update it
+                    retValue = value.original.value
+                    (value as Binding<T>).ourValue = retValue
+                    retValue
+                } else {
+                    value.ourValue as T
+                }
+            } else {
+                value as T
+            }
         }
         set(newValue) {
             while (!casValue(value, newValue)) Thread.yield()
         }
 
     override fun bindTo(sample: Property<T>) {
-        val newValue: Any?
-        val newSample: Property<T>?
-        if (sample.mayChange) {
-            newValue = Binding(sample)
-            newSample = sample
-        } else {
-            newValue = sample.value
-            newSample = null
-        }
+        val newValOrBinding: Any? = if (sample.mayChange) Binding(sample, sample.value) else sample.value
+        val oldValOrBinding = valueUpdater().get(this)
 
-        var oldValue: Any?
-        while (true) {
-            oldValue = valueUpdater().getAndSet(this, rebinding())
-            if (oldValue === rebinding()) Thread.yield() // other thread rebinding this property, wait
-            else break
-        }
-        // under mutex
+        val prevValue = if (oldValOrBinding is Binding<*>) (oldValOrBinding as Binding<T>).original.value else oldValOrBinding as T
+        val newValue = if (newValOrBinding is Binding<*>) (newValOrBinding as Binding<T>).original.value else newValOrBinding as T
 
-        // fixme: potential concurrent bug, #40
-        if ((oldValue !is Binding<*> || oldValue.original !== newSample) && isBeingObserved()) {
-            (oldValue as? Binding<T>)?.original?.removeChangeListener(this)
-            newSample?.addUnconfinedChangeListener(this)
+        withLockedTransition { // 'observed' state should not be changed concurrently
+            if (isBeingObserved()) {
+                (oldValOrBinding as? Binding<T>)?.original?.removeChangeListener(this)
+                (newValOrBinding as? Binding<T>)?.original?.addUnconfinedChangeListener(this)
+            }
+            (newValOrBinding as? Binding<T>)?.ourValue = newValue
+            valueUpdater().set(this, newValOrBinding)
         }
-        // end mutex
-        check(valueUpdater().getAndSet(this, newValue) === rebinding())
 
-        val prevValue = if (oldValue is Binding<*>) (oldValue as Binding<T>).original.value else oldValue as T
-        val newValueValue = if (newValue is Binding<*>) (newValue as Binding<T>).original.value else newValue as T
-
-        valueChanged(prevValue, newValueValue, null)
+        valueChanged(prevValue, newValue, null)
     }
 
     override fun observedStateChanged(observed: Boolean) {
@@ -70,48 +68,45 @@ internal class `ConcMutable-`<T>(
 
     override fun casValue(expect: T, update: T): Boolean {
         val prevValOrBind = valueUpdater().get(this)
-        if (prevValOrBind === rebinding()) return false
 
         val prevValue: T
         val realExpect: Any?
         if (prevValOrBind is Binding<*>) {
             prevValOrBind as Binding<T>
-            if (!valueUpdater().compareAndSet(this, prevValOrBind, rebinding())) return false
+            if (!tryLockTransition()) return false
 
             // under mutex
-            prevValue = prevValOrBind.original.value
+            prevValue = prevValOrBind.ourValue as T // hmm, where's my smart-cast?
             if (prevValue !== expect) {
                 // under mutex (no update from Sample allowed) we understand that sample's value !== expected
-                check(valueUpdater().compareAndSet(this, rebinding(), prevValOrBind))
-                // so just revert and report failure
+                unlockTransition()
+                // so just report failure
                 return false
             }
-            realExpect = rebinding()
+
             prevValOrBind.original.removeChangeListener(this)
+            unlockTransition()
+            realExpect = prevValOrBind
         } else {
             prevValue = prevValOrBind as T
             realExpect = expect
         }
 
-        val success = valueUpdater().compareAndSet(this, realExpect, update) // end mutex
+        val success = valueUpdater().compareAndSet(this, realExpect, update)
         if (success) {
             valueChanged(prevValue, update, null)
-        } else if (realExpect === rebinding()) {
-            // if we're set a mutex, we must release it; if it is not set, there's a program error
-            throw AssertionError()
         }
         return success
     }
 
     override fun invoke(old: T, new: T) {
-        while (value === rebinding()) Thread.yield()
-        valueChanged(old, new, null)
+        withLockedTransition {
+            (valueUpdater().get(this) as? Binding<T>)?.ourValue = new
+            valueChanged(old, new, null)
+        }
     }
 
-    private class Binding<T>(val original: Property<T>)
-
-    @Suppress("NOTHING_TO_INLINE")
-    private inline fun rebinding() = Unset
+    private class Binding<T>(val original: Property<T>, @Volatile var ourValue: T)
 
     @Suppress("NOTHING_TO_INLINE", "UNCHECKED_CAST")
     private inline fun valueUpdater() =

properties/src/main/kotlin/net/aquadc/properties/internal/utils.kt

@@ -74,7 +74,7 @@ val Unset: Any = Any()
         emptyArray<Any?>()
 
 @[JvmField JvmSynthetic] internal val NoListeners =
-        ConcListeners(EmptyArray, EmptyArray, false, false) as ConcListeners<Nothing, Nothing>
+        ConcListeners(EmptyArray, EmptyArray, false, false, false) as ConcListeners<Nothing, Nothing>
 
 @[JvmField JvmSynthetic PublishedApi] internal val TRUE = `Immutable-`(true)
 @[JvmField JvmSynthetic PublishedApi] internal val FALSE = `Immutable-`(false)