TVar: switch to lazy writes to provide proper isolation.

Author: chrisseaton

doc/tvar.md

@@ -24,7 +24,7 @@ We implement nested transactions by flattening.
 We only support strong isolation if you use the API correctly. In order words,
 we do not support strong isolation.
 
-Our implementation uses a very simple two-phased locking with versioned locks algorithm, as per [1]. In the future we will look at more advanced algorithms, contention management and using existing Java implementations when in JRuby.
+Our implementation uses a very simple two-phased locking with versioned locks algorithm and lazy writes, as per [1]. In the future we will look at more advanced algorithms, contention management and using existing Java implementations when in JRuby.
 
 See:
 

lib/concurrent/tvar.rb

@@ -153,14 +153,12 @@ class Transaction
     ABORTED = Object.new
 
     ReadLogEntry = Struct.new(:tvar, :version)
-    UndoLogEntry = Struct.new(:tvar, :value)
 
     AbortError = Class.new(StandardError)
 
     def initialize
-      @write_set = Set.new
       @read_log  = []
-      @undo_log  = []
+      @write_log = {}
     end
 
     def read(tvar)
@@ -172,18 +170,14 @@ def read(tvar)
     def write(tvar, value)
       # Have we already written to this TVar?
 
-      unless @write_set.include? tvar
+      unless @write_log.has_key? tvar
         # Try to lock the TVar
 
         unless tvar.unsafe_lock.try_lock
           # Someone else is writing to this TVar - abort
           Concurrent::abort_transaction
         end
 
-        # We've locked it - add it to the write set
-
-        @write_set.add(tvar)
-
         # If we previously wrote to it, check the version hasn't changed
 
         @read_log.each do |log_entry|
@@ -193,27 +187,20 @@ def write(tvar, value)
         end
       end
 
-      # Record the current value of the TVar so we can undo it later
-
-      @undo_log.push(UndoLogEntry.new(tvar, tvar.unsafe_value))
+      # Record the value written
 
-      # Write the new value to the TVar
-
-      tvar.unsafe_value = value
+      @write_log[tvar] = value
     end
 
     def abort
-      @undo_log.each do |entry|
-        entry.tvar.unsafe_value = entry.value
-      end
-
       unlock
     end
 
     def commit
       return false unless valid?
 
-      @write_set.each do |tvar|
+      @write_log.each_pair do |tvar, value|
+        tvar.unsafe_value = value
         tvar.unsafe_increment_version
       end
 
@@ -224,7 +211,7 @@ def commit
 
     def valid?
       @read_log.each do |log_entry|
-        unless @write_set.include? log_entry.tvar
+        unless @write_log.has_key? log_entry.tvar
           if log_entry.tvar.unsafe_version > log_entry.version
             return false
           end
@@ -235,7 +222,7 @@ def valid?
     end
 
     def unlock
-      @write_set.each do |tvar|
+      @write_log.each_key do |tvar|
         tvar.unsafe_lock.unlock
       end
     end

spec/concurrent/tvar_spec.rb

@@ -106,19 +106,44 @@ module Concurrent
       expect(t2.value).to eq 0
     end
 
-    it 'provides isolation' do
+    it 'provides weak isolation' do
       t = TVar.new(0)
 
+      a = CountDownLatch.new
+      b = CountDownLatch.new
+
       Thread.new do
         Concurrent::atomically do
-          t1.value = 1
-          sleep(1)
+          t.value = 1
+          a.count_down
+          b.wait
         end
       end
 
-      sleep(0.5)
+      a.wait
+      Concurrent::atomically do
+        expect(t.value).to eq 0
+      end
+      b.count_down
+    end
+
+    it 'provides strong isolation' do
+      t = TVar.new(0)
+
+      a = CountDownLatch.new
+      b = CountDownLatch.new
+
+      Thread.new do
+        Concurrent::atomically do
+          t.value = 1
+          a.count_down
+          b.wait
+        end
+      end
 
+      a.wait
       expect(t.value).to eq 0
+      b.count_down
     end
 
     it 'nests' do