From 6e202218ce433a1214cd4076ebff92159f23da39 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Konstantin=20K=C3=A4fer?= <mail@kkaefer.com>
Date: Fri, 30 Aug 2019 19:30:44 +0200
Subject: [PATCH] use randomly generated task IDs to avoid collisions when
 receiving messages from multiple workers (#8710)

Previously, we were using a monotonically increasing ID that started out with 0. This means there was a high chance of duplicate task IDs being processed at the same time when different execution contexts (aka web workers) sent messages to the another context. Since we store tasks keyed by task ID, tasks that were sent later overwrote tasks with the same ID that were already waiting in the queue. In practice, this only affected messages sent from a worker to the main thread. In most browsers, processing these tasks on the main thread is very quick, so they don't spend a lot of time in the queue. While this bug affects all browsers, Internet Explorer in particular isn't the fastest when processing messages, so had the highest likelihood of message ID collisions actually overwriting tasks waiting in the queue. We didn't observe this bug while testing on CI because we fake the web worker environment there, and task IDs could never be duplicated because they all ran in the same execution context.
---
 src/util/actor.js | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/src/util/actor.js b/src/util/actor.js
index af626c8ec30..8ca74db6649 100644
--- a/src/util/actor.js
+++ b/src/util/actor.js
@@ -29,8 +29,6 @@ class Actor {
     cancelCallbacks: { number: Cancelable };
     invoker: ThrottledInvoker;
 
-    static taskId: number;
-
     constructor(target: any, parent: any, mapId: ?number) {
         this.target = target;
         this.parent = parent;
@@ -53,7 +51,11 @@ class Actor {
      * @private
      */
     send(type: string, data: mixed, callback: ?Function, targetMapId: ?string): ?Cancelable {
-        const id = ++Actor.taskId;
+        // We're using a string ID instead of numbers because they are being used as object keys
+        // anyway, and thus stringified implicitly. We use random IDs because an actor may receive
+        // message from multiple other actors which could run in different execution context. A
+        // linearly increasing ID could produce collisions.
+        const id = Math.round((Math.random() * 1e18)).toString(36).substring(0, 10);
         if (callback) {
             this.callbacks[id] = callback;
         }
@@ -192,6 +194,4 @@ class Actor {
     }
 }
 
-Actor.taskId = 0;
-
 export default Actor;