fix(meteor-promise): create pool fibers without inherited async context

npm-packages/meteor-promise/fiber_pool.js

@@ -1,6 +1,13 @@
 var assert = require("assert");
 const { AsyncResource } = require("async_hooks");
 
+// Pool fibers are shared infrastructure: they must not inherit the AsyncLocalStorage stores
+// (OpenTelemetry context, etc.) of whichever caller happened to create them, because the
+// fiber's own async scope is what is active while it is parked between jobs. This resource is
+// created at module load, before any request context exists, so anything created inside it
+// starts with no stores at all.
+const PRISTINE_ASYNC_SCOPE = new AsyncResource("FiberPoolRoot");
+
 function FiberPool(targetFiberCount) {
   assert.ok(this instanceof FiberPool);
   assert.strictEqual(typeof targetFiberCount, "number");
@@ -12,10 +19,13 @@ function FiberPool(targetFiberCount) {
     // with our processing of the callback queue.
     var originalYield = Fiber.yield;
 
-    var fiber = new Fiber(function () {
+    var fiber = PRISTINE_ASYNC_SCOPE.runInAsyncScope(() => new Fiber(function () {
       while (fiber) {
-        // Call Fiber.yield() to await further instructions.
+        // Call Fiber.yield() to await further instructions. Flag the fiber as idle so that
+        // instrumentation wrapping Fiber.yield can tell this park apart from a real yield.
+        fiber._meteorPromisePoolIdle = true;
         var entry = originalYield.call(Fiber);
+        delete fiber._meteorPromisePoolIdle;
 
         if (! (entry &&
                typeof entry.callback === "function" &&
@@ -64,7 +74,7 @@ function FiberPool(targetFiberCount) {
           return;
         }
       }
-    });
+    }));
 
     // Run the new Fiber up to the first yield point, so that it will be
     // ready to receive entries.

npm-packages/meteor-promise/package.json

@@ -1,7 +1,7 @@
 {
   "name": "meteor-promise",
   "author": "Ben Newman <ben@meteor.com>",
-  "version": "0.9.1-2",
+  "version": "0.9.1-3",
   "description": "ES6 Promise polyfill with Fiber support",
   "keywords": [
     "meteor",

npm-packages/meteor-promise/test/tests.js

@@ -409,3 +409,75 @@ describe("stack traces", function () {
     });
   }));
 });
+
+describe("fiber pool async context", function () {
+  var AsyncLocalStorage = require("async_hooks").AsyncLocalStorage;
+  var als = new AsyncLocalStorage();
+
+  // Pool fibers are shared between unrelated callers. Between jobs each pool fiber parks
+  // itself with Fiber.yield() in its own async scope, so that scope must not carry the
+  // AsyncLocalStorage stores of whichever caller happened to create the fiber.
+  //
+  // Only meaningful with native promises: the `promise` polyfill schedules reactions through
+  // asap, which batches unrelated callbacks under a single async context.
+  var itWithNativePromise = Promise === global.Promise ? it : it.skip;
+  itWithNativePromise("does not expose the creator's ALS store at the idle yield", function () {
+    var GROW = 30; // > targetFiberCount, so new pool fibers must be created under "creator"
+    var USE = 50;
+    var recording = false;
+    var seenAtYields = [];
+    var seenAtIdleYields = [];
+    var originalYield = Fiber.yield;
+    Fiber.yield = function () {
+      if (recording) {
+        seenAtYields.push(als.getStore());
+        if (Fiber.current && Fiber.current._meteorPromisePoolIdle) {
+          seenAtIdleYields.push(als.getStore());
+        }
+      }
+      return originalYield.apply(this, arguments);
+    };
+
+    return als.run("creator", function () {
+      return Promise.all(Array.from({ length: GROW }, function () {
+        return Promise.asyncApply(function () { Promise.await(wait(20)); });
+      }));
+    }).then(function () {
+      recording = true;
+      return als.run("user", function () {
+        var chain = Promise.resolve();
+        var seenInsideJobs = [];
+        for (var i = 0; i < USE; ++i) {
+          chain = chain.then(function () {
+            return Promise.asyncApply(function () { return als.getStore(); });
+          }).then(function (store) { seenInsideJobs.push(store); });
+        }
+        return chain.then(function () { return seenInsideJobs; });
+      });
+    }).then(function (seenInsideJobs) {
+      recording = false;
+      Fiber.yield = originalYield;
+
+      // Inside a job the store is the submitter's.
+      assert.strictEqual(seenInsideJobs.length, USE);
+      seenInsideJobs.forEach(function (store) { assert.strictEqual(store, "user"); });
+
+      // No yield made while only "user" was running may see "creator". Before the fix every
+      // pool fiber's idle yield did, because the fiber inherited its creator's stores.
+      assert.ok(seenAtYields.length >= USE, "expected at least one yield per job, saw " + seenAtYields.length);
+      seenAtYields.forEach(function (store) {
+        assert.notStrictEqual(store, "creator", "a yield during the user phase saw the creator's store");
+      });
+
+      // The pool flags its idle yield, and that yield carries no store at all.
+      assert.ok(seenAtIdleYields.length >= USE, "expected a flagged idle yield per job, saw " + seenAtIdleYields.length);
+      seenAtIdleYields.forEach(function (store) {
+        assert.strictEqual(store, undefined, "idle yield saw store " + JSON.stringify(store));
+      });
+    }, function (error) {
+      recording = false;
+      Fiber.yield = originalYield;
+      throw error;
+    });
+  });
+});

packages/promise/package.js

@@ -1,14 +1,14 @@
 Package.describe({
   name: "promise",
-  version: "0.12.2",
+  version: "0.12.3",
   summary: "ECMAScript 2015 Promise polyfill with Fiber support",
   git: "https://github.com/meteor/promise",
   documentation: "README.md"
 });
 
 Npm.depends({
   // TODO: can we just drop this now?
-  "meteor-promise": "0.9.1-2",
+  "meteor-promise": "0.9.1-3",
   "promise": "8.1.0"
 });