enforce max_counter violations on send and recv

Author: langmartin

lib/hlclock/server.ex

@@ -34,8 +34,13 @@ defmodule HLClock.Server do
   end
 
   def handle_call(:send_timestamp, _from, data) do
-    {:ok, timestamp} = Timestamp.send(data.timestamp, physical_time())
-    {:reply, {:ok, timestamp}, %{data | timestamp: timestamp}}
+    case Timestamp.send(data.timestamp, physical_time()) do
+      {:ok, timestamp} ->
+        {:reply, {:ok, timestamp}, %{data | timestamp: timestamp}}
+
+      {:error, error} ->
+        {:reply, {:error, error}, data}
+    end
   end
 
   def handle_call(
@@ -55,8 +60,13 @@ defmodule HLClock.Server do
   def handle_info(:periodic_send, data) do
     Process.send_after(self(), :periodic_send, interval(data))
 
-    {:ok, ts} = Timestamp.send(data.timestamp, physical_time(), data.max_drift)
-    {:noreply, %{data | timestamp: ts}}
+    case Timestamp.send(data.timestamp, physical_time()) do
+      {:ok, ts} ->
+        {:noreply, %{data | timestamp: ts}}
+
+      {:error, _} ->
+        {:noreply, data}
+    end
   end
 
   defp physical_time, do: System.os_time(:millisecond)

lib/hlclock/timestamp.ex

@@ -46,7 +46,11 @@ defmodule HLClock.Timestamp do
   def send(%{time: old_time, counter: counter, node_id: node_id}, pt) do
     new_time = max(old_time, pt)
     new_counter = advance_counter(old_time, counter, new_time)
-    {:ok, new(new_time, new_counter, node_id)}
+
+    case handle_counter(new_counter) do
+      :ok -> {:ok, new(new_time, new_counter, node_id)}
+      err -> err
+    end
   end
 
   # Compatibility for older users of Timestamp that may be providing the max_drift.
@@ -67,15 +71,9 @@ defmodule HLClock.Timestamp do
     new_time = Enum.max([physical_time, local.time, remote.time])
 
     with {:ok, node_id} <- compare_node_ids(local.node_id, remote.node_id),
-         :ok <-
-           handle_drift(
-             remote.time,
-             physical_time,
-             max_drift,
-             :remote_drift_violation
-           ),
-         :ok <- handle_drift(new_time, physical_time, max_drift),
-         new_counter <- merge_logical(new_time, local, remote) do
+         :ok <- handle_drift(remote.time, physical_time, max_drift),
+         new_counter <- merge_logical(new_time, local, remote),
+         :ok <- handle_counter(new_counter) do
       {:ok, new(new_time, new_counter, node_id)}
     end
   end
@@ -183,10 +181,10 @@ defmodule HLClock.Timestamp do
     end
   end
 
-  defp handle_drift(l, pt, max_drift, err \\ :clock_drift_violation) do
+  defp handle_drift(l, pt, max_drift) do
     cond do
       drift?(l, pt, max_drift) ->
-        {:error, err}
+        {:error, :remote_drift_violation}
 
       true ->
         :ok
@@ -207,6 +205,14 @@ defmodule HLClock.Timestamp do
     end
   end
 
+  defp handle_counter(counter) do
+    if counter > 0xFFFF do
+      {:error, :max_counter_violation}
+    else
+      :ok
+    end
+  end
+
   defimpl String.Chars do
     def to_string(ts) do
       logical_time =

test/hlclock/timestamp_test.exs

@@ -91,7 +91,7 @@ defmodule HLClock.TimestampTest do
     property "for a fixed physical time, logical counter incremented" do
       check all(time <- ntp_millis()) do
         t0 = Timestamp.new(time, 0)
-        {:ok, t1} = Timestamp.send(t0, 0, @max_drift)
+        {:ok, t1} = Timestamp.send(t0, 0)
         assert t0.counter == 0
         assert t1.counter == 1
         refute Timestamp.before?(t1, t0)
@@ -100,10 +100,15 @@ defmodule HLClock.TimestampTest do
 
     test "physical time can move backwards" do
       t0 = Timestamp.new(10, 0, 0)
-      {:ok, t1} = Timestamp.send(t0, 9, @max_drift)
+      {:ok, t1} = Timestamp.send(t0, 9)
       assert t0.time == t1.time
       assert t1.counter == 1
     end
+
+    test "max counter exception" do
+      t0 = Timestamp.new(10, 0xFFFF, 0)
+      assert {:error, :max_counter_violation} == Timestamp.send(t0, 9)
+    end
   end
 
   describe "recv_timestamp/2" do
@@ -116,6 +121,15 @@ defmodule HLClock.TimestampTest do
       assert t2.node_id == 0
     end
 
+    test "max counter exception" do
+      t0 = Timestamp.new(10, 0xFFFF, 0)
+      remote = Timestamp.new(9, 0xFFFF, 1)
+      pt = 9
+
+      assert {:error, :max_counter_violation} ==
+               Timestamp.recv(t0, remote, pt, @max_drift)
+    end
+
     test "events test" do
       events = [
         # valid steps