Fix Speedtest worker lifecycle cleanup

package-lock.json

@@ -203,9 +203,9 @@
       }
     },
     "node_modules/ajv": {
-      "version": "6.12.6",
-      "resolved": "https://registry.npmjs.org/ajv/-/ajv-6.12.6.tgz",
-      "integrity": "sha512-j3fVLgvTo527anyYyJOGTYJbG+vnnQYvE0m5mmkc1TK+nxAppkCLMIL0aZ4dblVCNoGShhm+kzE4ZUykBoMg4g==",
+      "version": "6.15.0",
+      "resolved": "https://registry.npmjs.org/ajv/-/ajv-6.15.0.tgz",
+      "integrity": "sha512-fgFx7Hfoq60ytK2c7DhnF8jIvzYgOMxfugjLOSMHjLIPgenqa7S7oaagATUq99mV6IYvN2tRmC0wnTYX6iPbMw==",
       "dev": true,
       "license": "MIT",
       "dependencies": {
@@ -260,9 +260,9 @@
       "license": "MIT"
     },
     "node_modules/brace-expansion": {
-      "version": "1.1.16",
-      "resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-1.1.16.tgz",
-      "integrity": "sha512-IDw48K2/2kRkg9LdJxurvq3lV3aBgq0REY89duEqFRthjlPdXHKMj7EnQOXVckxzgisinf3nHfrcE2FufFLXMw==",
+      "version": "1.1.18",
+      "resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-1.1.18.tgz",
+      "integrity": "sha512-Edep/X9fGqVNmzKBVsDYIOtD+z1tuezV70LBjdCst9Tqu76lsnvRiZ6oTic1n+/BIwX6QDGAO94PN4N2SADvtw==",
       "dev": true,
       "license": "MIT",
       "dependencies": {
@@ -811,9 +811,9 @@
       "license": "ISC"
     },
     "node_modules/js-yaml": {
-      "version": "4.3.1",
-      "resolved": "https://registry.npmjs.org/js-yaml/-/js-yaml-4.3.1.tgz",
-      "integrity": "sha512-CY6crGq313MX8GkwvB7tzgp99vjQxY1++5y10/BKN/GUfHqWaOGQMNZkBvqSzsZKWk/ijwHlWzzkLulsGHhjWQ==",
+      "version": "4.3.2",
+      "resolved": "https://registry.npmjs.org/js-yaml/-/js-yaml-4.3.2.tgz",
+      "integrity": "sha512-SFNOvSJ+Dgf/9An904Yx+CgSlIPCkIpao4qo51lpee25TIRejdH3rhR4EZMGoNx3/TP3O+wzWuiTFl4sqbltzA==",
       "dev": true,
       "funding": [
         {

speedtest.js

@@ -48,6 +48,9 @@ function Speedtest() {
   this._selectedServer = null; //when using multiple points of test, this is the selected server
   this._settings = {}; //settings for the speed test worker
   this._state = 0; //0=adding settings, 1=adding servers, 2=server selection done, 3=test running, 4=done
+  this.worker = null;
+  this.updater = null;
+  this._activeRun = null;
   console.log(
     "LibreSpeed by Federico Dossena v6.2.1 - https://github.com/librespeed/speedtest"
   );
@@ -319,34 +322,68 @@ Speedtest.prototype = {
    */
   start: function() {
     if (this._state == 3) throw "Test already running";
-    this.worker = new Worker("speedtest_worker.js?r=" + Math.random());
-    this.worker.onmessage = function(e) {
-      if (e.data === this._prevData) return;
-      else this._prevData = e.data;
+    if (this._state == 1)
+      throw "When using multiple points of test, you must call selectServer before starting the test";
+
+    const worker = new Worker("speedtest_worker.js?r=" + Math.random());
+    const run = {
+      worker: worker,
+      updater: null,
+      abortTimeout: null,
+      ended: false,
+      prevData: null
+    };
+    const isCurrentRun = function() {
+      return this._activeRun === run && this.worker === worker;
+    }.bind(this);
+    const finish = function(aborted) {
+      if (run.ended) return;
+      run.ended = true;
+      if (run.updater !== null) {
+        clearInterval(run.updater);
+        run.updater = null;
+      }
+      if (run.abortTimeout !== null) {
+        clearTimeout(run.abortTimeout);
+        run.abortTimeout = null;
+      }
+      worker.terminate();
+
+      // A callback may synchronously start another run, so clean up this run first.
+      if (!isCurrentRun()) return;
+      this._activeRun = null;
+      this.worker = null;
+      this.updater = null;
+      this._state = 4;
+      try {
+        if (this.onend) this.onend(aborted);
+      } catch (e) {
+        console.error("Speedtest onend event threw exception: " + e);
+      }
+    }.bind(this);
+
+    run.finish = finish;
+    this._activeRun = run;
+    this.worker = worker;
+    worker.onmessage = function(e) {
+      if (!isCurrentRun() || run.ended) return;
+      if (e.data === run.prevData) return;
+      else run.prevData = e.data;
       const data = JSON.parse(e.data);
       try {
         if (this.onupdate) this.onupdate(data);
       } catch (e) {
         console.error("Speedtest onupdate event threw exception: " + e);
       }
-      if (data.testState >= 4) {
-        clearInterval(this.updater);
-        this._state = 4;
-        try {
-          if (this.onend) this.onend(data.testState == 5);
-        } catch (e) {
-          console.error("Speedtest onend event threw exception: " + e);
-        }
-      }
+      if (data.testState >= 4) finish(data.testState == 5);
     }.bind(this);
-    this.updater = setInterval(
+    run.updater = setInterval(
       function() {
-        this.worker.postMessage("status");
-      }.bind(this),
+        if (isCurrentRun() && !run.ended) worker.postMessage("status");
+      },
       200
     );
-    if (this._state == 1)
-        throw "When using multiple points of test, you must call selectServer before starting the test";
+    this.updater = run.updater;
     if (this._state == 2) {
       this._settings.url_dl =
         this._selectedServer.server + this._selectedServer.dlURL;
@@ -367,13 +404,22 @@ Speedtest.prototype = {
         });
     }
     this._state = 3;
-    this.worker.postMessage("start " + JSON.stringify(this._settings));
+    worker.postMessage("start " + JSON.stringify(this._settings));
   },
   /**
    * Aborts the test while it's running.
    */
   abort: function() {
     if (this._state < 3) throw "You cannot abort a test that's not started yet";
-    if (this._state < 4) this.worker.postMessage("abort");
+    const run = this._activeRun;
+    if (this._state < 4 && run && this.worker === run.worker) {
+      run.worker.postMessage("abort");
+      if (!run.ended && run.abortTimeout === null) {
+        run.abortTimeout = setTimeout(function() {
+          if (this._activeRun === run && this.worker === run.worker)
+            run.finish(true);
+        }.bind(this), 1000);
+      }
+    }
   }
 };

tests/e2e/worker-lifecycle.spec.js

@@ -0,0 +1,224 @@
+const fs = require("node:fs");
+const path = require("node:path");
+const { test, expect } = require("@playwright/test");
+
+const speedtestSource = fs.readFileSync(
+  path.join(__dirname, "..", "..", "speedtest.js"),
+  "utf8"
+);
+
+async function installSpeedtest(page) {
+  await page.goto("about:blank");
+  await page.evaluate(() => {
+    window.__workers = [];
+    window.Worker = class FakeWorker {
+      constructor(url) {
+        this.url = url;
+        this.messages = [];
+        this.terminated = false;
+        this.terminateCount = 0;
+        window.__workers.push(this);
+      }
+
+      postMessage(message) {
+        this.messages.push(message);
+      }
+
+      terminate() {
+        this.terminated = true;
+        this.terminateCount++;
+      }
+
+      emit(data) {
+        this.onmessage(new MessageEvent("message", { data: JSON.stringify(data) }));
+      }
+    };
+  });
+  await page.addScriptTag({ content: speedtestSource });
+}
+
+function terminalState(testState) {
+  return { testState: testState };
+}
+
+test.describe("Speedtest worker lifecycle", () => {
+  test.beforeEach(async ({ page }) => {
+    await installSpeedtest(page);
+  });
+
+  test("terminates a worker after normal completion", async ({ page }) => {
+    const result = await page.evaluate((data) => {
+      const speedtest = new Speedtest();
+      speedtest.start();
+      const worker = window.__workers[0];
+      worker.emit(data);
+      return {
+        state: speedtest.getState(),
+        worker: speedtest.worker,
+        updater: speedtest.updater,
+        terminated: worker.terminated,
+        terminateCount: worker.terminateCount
+      };
+    }, terminalState(4));
+
+    expect(result).toEqual({
+      state: 4,
+      worker: null,
+      updater: null,
+      terminated: true,
+      terminateCount: 1
+    });
+  });
+
+  test("can start again after normal completion", async ({ page }) => {
+    const result = await page.evaluate((data) => {
+      const speedtest = new Speedtest();
+      speedtest.start();
+      const first = window.__workers[0];
+      first.emit(data);
+      speedtest.start();
+      const second = window.__workers[1];
+      return {
+        state: speedtest.getState(),
+        firstTerminated: first.terminated,
+        secondTerminated: second.terminated,
+        activeWorkerIsSecond: speedtest.worker === second
+      };
+    }, terminalState(4));
+
+    expect(result).toEqual({
+      state: 3,
+      firstTerminated: true,
+      secondTerminated: false,
+      activeWorkerIsSecond: true
+    });
+  });
+
+  test("can start again after an abort response", async ({ page }) => {
+    const result = await page.evaluate((data) => {
+      const speedtest = new Speedtest();
+      speedtest.start();
+      const first = window.__workers[0];
+      speedtest.abort();
+      first.emit(data);
+      speedtest.start();
+      const second = window.__workers[1];
+      return {
+        state: speedtest.getState(),
+        firstTerminated: first.terminated,
+        secondTerminated: second.terminated,
+        activeWorkerIsSecond: speedtest.worker === second
+      };
+    }, terminalState(5));
+
+    expect(result).toEqual({
+      state: 3,
+      firstTerminated: true,
+      secondTerminated: false,
+      activeWorkerIsSecond: true
+    });
+  });
+
+  test("does not terminate a worker started synchronously by onend", async ({ page }) => {
+    const result = await page.evaluate((data) => {
+      const speedtest = new Speedtest();
+      let endCalls = 0;
+      speedtest.onend = () => {
+        endCalls++;
+        speedtest.start();
+      };
+      speedtest.start();
+      const first = window.__workers[0];
+      first.emit(data);
+      const second = window.__workers[1];
+      return {
+        endCalls: endCalls,
+        state: speedtest.getState(),
+        firstTerminated: first.terminated,
+        secondTerminated: second.terminated,
+        activeWorkerIsSecond: speedtest.worker === second
+      };
+    }, terminalState(4));
+
+    expect(result).toEqual({
+      endCalls: 1,
+      state: 3,
+      firstTerminated: true,
+      secondTerminated: false,
+      activeWorkerIsSecond: true
+    });
+  });
+
+  test("forces a single aborted completion when a worker does not respond", async ({ page }) => {
+    await page.evaluate(() => {
+      window.__speedtest = new Speedtest();
+      window.__endCalls = 0;
+      window.__speedtest.onend = (aborted) => {
+        if (aborted) window.__endCalls++;
+      };
+      window.__speedtest.start();
+      window.__speedtest.abort();
+    });
+
+    await expect
+      .poll(() =>
+        page.evaluate(() => ({
+          state: window.__speedtest.getState(),
+          worker: window.__speedtest.worker,
+          updater: window.__speedtest.updater,
+          terminated: window.__workers[0].terminated,
+          endCalls: window.__endCalls
+        }))
+      )
+      .toEqual({
+        state: 4,
+        worker: null,
+        updater: null,
+        terminated: true,
+        endCalls: 1
+      });
+
+    await page.waitForTimeout(1100);
+    expect(await page.evaluate(() => window.__endCalls)).toBe(1);
+  });
+
+  test("ignores delayed events and an old abort timeout after a new run starts", async ({ page }) => {
+    await page.evaluate((data) => {
+      window.__speedtest = new Speedtest();
+      window.__endCalls = 0;
+      window.__updates = 0;
+      window.__speedtest.onupdate = () => window.__updates++;
+      window.__speedtest.onend = () => {
+        window.__endCalls++;
+        window.__speedtest.start();
+      };
+      window.__speedtest.start();
+      window.__first = window.__workers[0];
+      window.__speedtest.abort();
+      window.__first.emit(data);
+      window.__second = window.__workers[1];
+    }, terminalState(5));
+
+    await page.waitForTimeout(1100);
+    const result = await page.evaluate((data) => {
+      window.__first.emit(data);
+      return {
+        endCalls: window.__endCalls,
+        updates: window.__updates,
+        state: window.__speedtest.getState(),
+        firstTerminated: window.__first.terminated,
+        secondTerminated: window.__second.terminated,
+        activeWorkerIsSecond: window.__speedtest.worker === window.__second
+      };
+    }, terminalState(5));
+
+    expect(result).toEqual({
+      endCalls: 1,
+      updates: 1,
+      state: 3,
+      firstTerminated: true,
+      secondTerminated: false,
+      activeWorkerIsSecond: true
+    });
+  });
+});