From 5dae7d67589c908a1fe672b084838c1397d00e54 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Tobias=20Nie=C3=9Fen?= <tniessen@tnie.de>
Date: Wed, 9 Sep 2020 00:19:02 +0200
Subject: [PATCH] crypto: add buffering to randomInt

PR-URL: https://github.com/nodejs/node/pull/35110
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Andrey Pechkurov <apechkurov@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
---
 benchmark/crypto/randomInt.js | 38 +++++++++++++++
 lib/internal/crypto/random.js | 87 +++++++++++++++++++++++++----------
 2 files changed, 100 insertions(+), 25 deletions(-)
 create mode 100644 benchmark/crypto/randomInt.js

diff --git a/benchmark/crypto/randomInt.js b/benchmark/crypto/randomInt.js
new file mode 100644
index 00000000000000..1f1dfa0160fa11
--- /dev/null
+++ b/benchmark/crypto/randomInt.js
@@ -0,0 +1,38 @@
+'use strict';
+
+const common = require('../common.js');
+const { randomInt } = require('crypto');
+
+const bench = common.createBenchmark(main, {
+  mode: ['sync', 'async-sequential', 'async-parallel'],
+  min: [-(2 ** 47) + 1, -10_000, -100],
+  max: [100, 10_000, 2 ** 47],
+  n: [1e3, 1e5]
+});
+
+function main({ mode, min, max, n }) {
+  if (mode === 'sync') {
+    bench.start();
+    for (let i = 0; i < n; i++)
+      randomInt(min, max);
+    bench.end(n);
+  } else if (mode === 'async-sequential') {
+    bench.start();
+    (function next(i) {
+      if (i === n)
+        return bench.end(n);
+      randomInt(min, max, () => {
+        next(i + 1);
+      });
+    })(0);
+  } else {
+    bench.start();
+    let done = 0;
+    for (let i = 0; i < n; i++) {
+      randomInt(min, max, () => {
+        if (++done === n)
+          bench.end(n);
+      });
+    }
+  }
+}
diff --git a/lib/internal/crypto/random.js b/lib/internal/crypto/random.js
index 161e1a21b36f85..88b535a357c4fa 100644
--- a/lib/internal/crypto/random.js
+++ b/lib/internal/crypto/random.js
@@ -2,6 +2,10 @@
 
 const {
   Array,
+  ArrayPrototypeForEach,
+  ArrayPrototypePush,
+  ArrayPrototypeShift,
+  ArrayPrototypeSplice,
   BigInt,
   FunctionPrototypeBind,
   FunctionPrototypeCall,
@@ -186,6 +190,13 @@ function randomFill(buf, offset, size, callback) {
 // e.g.: Buffer.from("ff".repeat(6), "hex").readUIntBE(0, 6);
 const RAND_MAX = 0xFFFF_FFFF_FFFF;
 
+// Cache random data to use in randomInt. The cache size must be evenly
+// divisible by 6 because each attempt to obtain a random int uses 6 bytes.
+const randomCache = new FastBuffer(6 * 1024);
+let randomCacheOffset = randomCache.length;
+let asyncCacheFillInProgress = false;
+const asyncCachePendingTasks = [];
+
 // Generates an integer in [min, max) range where min is inclusive and max is
 // exclusive.
 function randomInt(min, max, callback) {
@@ -230,33 +241,59 @@ function randomInt(min, max, callback) {
   // than or equal to 0 and less than randLimit.
   const randLimit = RAND_MAX - (RAND_MAX % range);
 
-  if (isSync) {
-    // Sync API
-    while (true) {
-      const x = randomBytes(6).readUIntBE(0, 6);
-      if (x >= randLimit) {
-        // Try again.
-        continue;
-      }
-      return (x % range) + min;
+  // If we don't have a callback, or if there is still data in the cache, we can
+  // do this synchronously, which is super fast.
+  while (isSync || (randomCacheOffset < randomCache.length)) {
+    if (randomCacheOffset === randomCache.length) {
+      // This might block the thread for a bit, but we are in sync mode.
+      randomFillSync(randomCache);
+      randomCacheOffset = 0;
+    }
+
+    const x = randomCache.readUIntBE(randomCacheOffset, 6);
+    randomCacheOffset += 6;
+
+    if (x < randLimit) {
+      const n = (x % range) + min;
+      if (isSync) return n;
+      process.nextTick(callback, undefined, n);
+      return;
     }
-  } else {
-    // Async API
-    const pickAttempt = () => {
-      randomBytes(6, (err, bytes) => {
-        if (err) return callback(err);
-        const x = bytes.readUIntBE(0, 6);
-        if (x >= randLimit) {
-          // Try again.
-          return pickAttempt();
-        }
-        const n = (x % range) + min;
-        callback(null, n);
-      });
-    };
-
-    pickAttempt();
   }
+
+  // At this point, we are in async mode with no data in the cache. We cannot
+  // simply refill the cache, because another async call to randomInt might
+  // already be doing that. Instead, queue this call for when the cache has
+  // been refilled.
+  ArrayPrototypePush(asyncCachePendingTasks, { min, max, callback });
+  asyncRefillRandomIntCache();
+}
+
+function asyncRefillRandomIntCache() {
+  if (asyncCacheFillInProgress)
+    return;
+
+  asyncCacheFillInProgress = true;
+  randomFill(randomCache, (err) => {
+    asyncCacheFillInProgress = false;
+
+    const tasks = asyncCachePendingTasks;
+    const errorReceiver = err && ArrayPrototypeShift(tasks);
+    if (!err)
+      randomCacheOffset = 0;
+
+    // Restart all pending tasks. If an error occurred, we only notify a single
+    // callback (errorReceiver) about it. This way, every async call to
+    // randomInt has a chance of being successful, and it avoids complex
+    // exception handling here.
+    ArrayPrototypeForEach(ArrayPrototypeSplice(tasks, 0), (task) => {
+      randomInt(task.min, task.max, task.callback);
+    });
+
+    // This is the only call that might throw, and is therefore done at the end.
+    if (errorReceiver)
+      errorReceiver.callback(err);
+  });
 }