From fe2d609078193e921b8605039d70191abc2171ef Mon Sep 17 00:00:00 2001
From: Jack Kleeman <jackkleeman@gmail.com>
Date: Thu, 15 Jun 2023 18:12:29 +0200
Subject: [PATCH] Avoid cyclical deadlocks in the verifier (#144)

* Avoid cyclical deadlocks in the verifier

By always incrementing targets when we are trying to hit something we know to be unlocked, we can avoid cycles in the waits-for graph

* Add comment for target
---
 contracts/src/main/proto/interpreter.proto |  1 +
 services/node-services/src/verifier.ts     | 21 +++++++--------------
 2 files changed, 8 insertions(+), 14 deletions(-)

diff --git a/contracts/src/main/proto/interpreter.proto b/contracts/src/main/proto/interpreter.proto
index 01f778db..fe32af01 100644
--- a/contracts/src/main/proto/interpreter.proto
+++ b/contracts/src/main/proto/interpreter.proto
@@ -31,6 +31,7 @@ message TestParams {
 
 message Key {
   TestParams params = 1;
+  // target is an arbitrary integer allowing us to select different service keys
   int32 target = 2;
 }
 
diff --git a/services/node-services/src/verifier.ts b/services/node-services/src/verifier.ts
index 9f9b717d..4a1fd5d4 100644
--- a/services/node-services/src/verifier.ts
+++ b/services/node-services/src/verifier.ts
@@ -22,7 +22,6 @@ import {
 import seedrandom from "seedrandom";
 import { useContext } from "@restatedev/restate-sdk";
 
-const MAX_TARGET = 1024;
 const DEFAULT_MAX_SLEEP = 32768;
 
 export class CommandBuilder {
@@ -38,15 +37,6 @@ export class CommandBuilder {
     return Math.floor(Math.abs(this.random() * max));
   }
 
-  randomTarget(...lockedTargets: Array<number>): number {
-    let target = this.randomInt(MAX_TARGET);
-    // rejection sampling
-    while (lockedTargets.includes(target)) {
-      target = this.randomInt(MAX_TARGET);
-    }
-    return target;
-  }
-
   normaliseSleeps(commands: Commands | undefined, factor: number) {
     if (commands == undefined) {
       return;
@@ -99,7 +89,7 @@ export class CommandBuilder {
     maxSleepMillis: number,
     depth: number
   ): { target: number; commands: Commands } {
-    const call = this._buildCommands(this.randomTarget(), depth, []);
+    const call = this._buildCommands(0, depth, []);
     const duration = this.durationUpperBound(call.commands);
     // normalise so that the entire job takes less time than the max sleep
     this.normaliseSleeps(call.commands, maxSleepMillis / duration);
@@ -133,7 +123,10 @@ export class CommandBuilder {
       () => ({
         // hit a known-unlocked target with a sync call, and pass on the lock list for future blocking calls
         syncCall: this._buildCommands(
-          this.randomTarget(target, ...lockedTargets),
+          // jump to a target between 1 and 32 ahead
+          // by only going upwards we avoid cycles
+          // by skipping up to 32 we avoid all paths landing on the same few keys
+          target + 1 + this.randomInt(32),
           depth - 1,
           [target, ...lockedTargets]
         ),
@@ -143,7 +136,7 @@ export class CommandBuilder {
           callId: asyncUnlockedCounter++,
           // hit a known-unlocked target with an async call that may be awaited, and pass on the lock list for future blocking calls
           ...this._buildCommands(
-            this.randomTarget(target, ...lockedTargets),
+            target + 1 + this.randomInt(32),
             depth - 1,
             [target, ...lockedTargets]
           ),
@@ -182,7 +175,7 @@ export class CommandBuilder {
       () => ({
         // deliberately hit a known-unlocked target with a background call (the call should schedule asap)
         backgroundCall: this._buildCommands(
-          this.randomTarget(target, ...lockedTargets),
+          target + 1 + this.randomInt(32),
           depth - 1,
           []
         ),