File: keep the FileReader listener snapshot rooted while dispatching

Polyfills/File/Source/FileReader.cpp

@@ -164,12 +164,12 @@ namespace Babylon::Polyfills::Internal
         auto& list = m_eventHandlerRefs[eventType];
         for (const auto& existing : list)
         {
-            if (existing.Value() == handler)
+            if (existing->callback.Value() == handler)
             {
                 return;
             }
         }
-        list.push_back(Napi::Persistent(handler));
+        list.push_back(std::make_shared<EventListener>(Napi::Persistent(handler)));
     }
 
     void FileReader::RemoveEventListener(const Napi::CallbackInfo& info)
@@ -191,8 +191,9 @@ namespace Babylon::Polyfills::Internal
         auto& list = it->second;
         for (auto i = list.begin(); i != list.end(); ++i)
         {
-            if (i->Value() == handler)
+            if ((*i)->callback.Value() == handler)
             {
+                (*i)->removed = true;
                 list.erase(i);
                 return;
             }
@@ -240,19 +241,19 @@ namespace Babylon::Polyfills::Internal
             return;
         }
 
-        // Snapshot the listener list so that mutations during dispatch
-        // (e.g. a handler that calls removeEventListener) do not invalidate
-        // the iterator we are walking.
-        std::vector<Napi::Function> snapshot;
-        snapshot.reserve(it->second.size());
-        for (const auto& ref : it->second)
-        {
-            snapshot.push_back(ref.Value());
-        }
+        // Share listener records: keep callbacks rooted without invalidating iteration,
+        // but observe removals even in nested dispatches. Re-adding a callback creates a
+        // new record that is not in this snapshot (DOM's invoke / inner invoke algorithms).
+        const auto snapshot = it->second;
 
         for (const auto& listener : snapshot)
         {
-            listener.Call(jsThis, {event});
+            if (listener->removed)
+            {
+                continue;
+            }
+
+            listener->callback.Value().Call(jsThis, {event});
             if (env.IsExceptionPending())
             {
                 env.GetAndClearPendingException();

Polyfills/File/Source/FileReader.h

@@ -3,6 +3,7 @@
 #include <napi/napi.h>
 
 #include <cstdint>
+#include <memory>
 #include <string>
 #include <unordered_map>
 #include <vector>
@@ -21,6 +22,12 @@ namespace Babylon::Polyfills::Internal
         explicit FileReader(const Napi::CallbackInfo& info);
 
     private:
+        struct EventListener
+        {
+            Napi::FunctionReference callback;
+            bool removed{false};
+        };
+
         enum class ReadMode
         {
             ArrayBuffer,
@@ -65,7 +72,7 @@ namespace Babylon::Polyfills::Internal
         // wrapper is kept alive by an externally-held anchor (see StartRead),
         // so `this` is always valid when a continuation reads this field.
         uint64_t m_readId{0};
-        std::unordered_map<std::string, std::vector<Napi::FunctionReference>> m_eventHandlerRefs;
+        std::unordered_map<std::string, std::vector<std::shared_ptr<EventListener>>> m_eventHandlerRefs;
 
         // readonly attribute state, surfaced through the getters above.
         int32_t m_readyState{EMPTY};

Tests/UnitTests/Scripts/tests.ts

@@ -2238,6 +2238,105 @@ describe("FileReader", function () {
         reader.readAsText(blob);
     });
 
+    it("skips a listener removed during dispatch and still calls the final listener", function (done) {
+        const reader = new FileReader();
+        const blob = new Blob(["abc"]);
+        const calls: string[] = [];
+        let laterListener: (() => void) | null = function () {
+            calls.push("removed");
+        };
+
+        reader.addEventListener("load", function () {
+            calls.push("first");
+            reader.removeEventListener("load", laterListener);
+            laterListener = null;
+
+            // Add allocation pressure for JSC_collectContinuously=1 runs. The assertion
+            // below is deterministic and does not depend on whether collection occurs.
+            const pressure = [];
+            for (let i = 0; i < 64; ++i) {
+                pressure.push(new Uint8Array(16 * 1024));
+            }
+            calls.push(`allocated ${pressure.length}`);
+        });
+        reader.addEventListener("load", laterListener);
+        reader.addEventListener("load", function () {
+            calls.push("last");
+        });
+        reader.onloadend = function () {
+            try {
+                expect(calls).to.deep.equal(["first", "allocated 64", "last"]);
+                done();
+            } catch (e) {
+                done(e);
+            }
+        };
+        reader.readAsText(blob);
+    });
+
+    it("defers a removed and re-added listener until the next dispatch", function () {
+        const reader = new FileReader();
+        const calls: string[] = [];
+        const later = () => calls.push("later");
+        const first = function () {
+            calls.push("first");
+            reader.removeEventListener("load", first);
+            reader.removeEventListener("load", later);
+            reader.addEventListener("load", later);
+        };
+        reader.addEventListener("load", first);
+        reader.addEventListener("load", later);
+        reader.addEventListener("load", () => calls.push("last"));
+
+        reader.dispatchEvent({ type: "load" });
+        expect(calls).to.deep.equal(["first", "last"]);
+        calls.length = 0;
+        reader.dispatchEvent({ type: "load" });
+        expect(calls).to.deep.equal(["last", "later"]);
+    });
+
+    it("allows self-removal and defers newly added listeners", function () {
+        const reader = new FileReader();
+        const calls: string[] = [];
+        const added = () => calls.push("added");
+        const first = function () {
+            calls.push("first");
+            reader.removeEventListener("load", first);
+            reader.addEventListener("load", added);
+        };
+        reader.addEventListener("load", first);
+        reader.addEventListener("load", () => calls.push("last"));
+
+        reader.dispatchEvent({ type: "load" });
+        expect(calls).to.deep.equal(["first", "last"]);
+        calls.length = 0;
+        reader.dispatchEvent({ type: "load" });
+        expect(calls).to.deep.equal(["last", "added"]);
+    });
+
+    it("observes removals made by a nested dispatch", function () {
+        const reader = new FileReader();
+        const calls: string[] = [];
+        const first = function () {
+            calls.push("first");
+            reader.removeEventListener("load", first);
+            reader.dispatchEvent({ type: "load" });
+        };
+        const middle = function () {
+            calls.push("middle");
+            reader.removeEventListener("load", middle);
+        };
+        reader.addEventListener("load", first);
+        reader.addEventListener("load", middle);
+        reader.addEventListener("load", () => calls.push("last"));
+
+        reader.dispatchEvent({ type: "load" });
+        expect(calls).to.deep.equal(["first", "middle", "last", "last"]);
+        calls.length = 0;
+        reader.dispatchEvent({ type: "load" });
+        expect(calls).to.deep.equal(["last"]);
+    });
+
     // -------------------------------- abort --------------------------------
     it("transitions readyState to DONE after abort()", function (done) {
         const reader = new FileReader();