From 78188249cabd2a7effffb737ec0909ac491df06b Mon Sep 17 00:00:00 2001 From: Samuel Degueldre Date: Mon, 19 Dec 2022 12:45:29 +0000 Subject: [PATCH] [FIX] bus, web: clean up event handlers and message ports correctly This commit fixes two things: - The owl compatibility layer adds an `on` and `off` method on owl's EventBus so that it can keep being used as it used to in owl 1. To do this, it needs to keep track of the the callbacks that are attached to it and by whom. To do this it keeps a Map where the "owners" are keys. When calling `off` to remove event listeners, we remove the callbacks but did not remove the owner from the Map, causing the EventBus to always hold a strong reference to any object that was given as the owner to the `on` method, preventing the item from being garbage collected. Objects that do this are typically components, and components hold a reference to their owl application that contains the entire tree of components. This was particularly problematic in tests where components are created and destroyed at a rapid pace. - The mock websocket creates a MessageChannel so that it can mock the server side of the websocket, but it never closes that MessageChannel's MessagePorts, which can cause garbage collection to be slow to happen [1] (in practice it looks like these are basically never garbage collected unless closed). This commit registers a cleanup to close the message ports after the execution of the current test. [1]: https://html.spec.whatwg.org/dev/web-messaging.html#ports-and-garbage-collection closes odoo/odoo#108309 X-original-commit: 4facc396a4e8036592a473002253d5a5daf41a19 Signed-off-by: Aaron Bohy (aab) Co-authored-by: Aaron Bohy --- .../bus/static/tests/helpers/mock_websocket.js | 7 ++++++- .../src/owl2_compatibility/event_target.js | 17 +++++++++++++---- 2 files changed, 19 insertions(+), 5 deletions(-) diff --git a/addons/bus/static/tests/helpers/mock_websocket.js b/addons/bus/static/tests/helpers/mock_websocket.js index ab5b04a2f1f..5fa2e7e66b9 100644 --- a/addons/bus/static/tests/helpers/mock_websocket.js +++ b/addons/bus/static/tests/helpers/mock_websocket.js @@ -73,7 +73,12 @@ export function patchWebsocketWorkerWithCleanup(params = {}) { websocketWorker = websocketWorker || new WebsocketWorker('wss://odoo.com/websocket'); patchWithCleanup(browser, { SharedWorker: function () { - return new SharedWorkerMock(websocketWorker); + const sharedWorker = new SharedWorkerMock(websocketWorker); + registerCleanup(() => { + sharedWorker._messageChannel.port1.close(); + sharedWorker._messageChannel.port2.close(); + }); + return sharedWorker; }, }, { pure: true }); registerCleanup(() => { diff --git a/addons/web/static/src/owl2_compatibility/event_target.js b/addons/web/static/src/owl2_compatibility/event_target.js index 8ef1c23ef29..868ea2a7e71 100644 --- a/addons/web/static/src/owl2_compatibility/event_target.js +++ b/addons/web/static/src/owl2_compatibility/event_target.js @@ -14,20 +14,29 @@ } on(type, target, callback) { if (!this.targetsCallbacks.has(target)) { - this.targetsCallbacks.set(target, []); + this.targetsCallbacks.set(target, {}); } callback = wrapCallback(target, callback); - this.targetsCallbacks.get(target).push(callback); + const listeners = this.targetsCallbacks.get(target); + if (!listeners[type]) { + listeners[type] = new Set(); + } + listeners[type].add(callback); return this.addEventListener(type, callback); } off(type, target) { - const cbs = this.targetsCallbacks.get(target); - if (!cbs) { + const listeners = this.targetsCallbacks.get(target); + if (!listeners || !Object.hasOwnProperty.call(listeners, type)) { return; } + const cbs = listeners[type]; for (const callback of cbs) { this.removeEventListener(type, callback); } + delete cbs[type]; + if (Object.keys(cbs).length === 0) { + this.targetsCallbacks.delete(target); + } } }; })();