Skip to content
Snippets Groups Projects
Commit 5241955a authored by Aaron Bohy's avatar Aaron Bohy
Browse files

[FIX] bus: prevent longpoll requests storm

Before this commit, it might happen that, in some situations,
with several tabs opened, the CrossTabBus called the longpolling
route repeatedly, thus slowing down the server, and freezing the
webclient.

The issue was tricky to reproduce. It was a race-condition that
could occur when several tabs performed simultanous calls to
addChannel, while being unloaded or becoming mastertab in the
meantime (e.g. when opening/closing/refreshing several tabs
simultaneously).

This issue has been introduced by [1] which by mistake (probably)
made each tab calling itself the localStorage to update the list
of channels when it was notified that the list of channels in
the localStorage just changed. So if several tabs had a slightly
different list of channels at a given moment (e.g. at startup),
it might happen that they in turn, undo what another tab just
put in the localStorage, and thus produced an infinite loop of
localStorage writes and longpolling request aborts/calls.

The issue could be reproduced with the OCA module [2], which
performs several addChannel at webclient startup.

This commit restores this part of the code as it was initially
written in [3].

Closes #69067

opw~2502799
maybe opw~2451865 as well

[1] https://github.com/odoo/odoo/commit/6448420
[2] https://odoo-community.org/shop/product/web-notify-2670#attr=10773
[3] https://github.com/odoo/odoo/commit/38581f67236377daa767ca2216529a26b8708b00#diff-f6eccad21ae3543606ab8f97b8b097d015412caeaee2bf8cc928eb3ccabac9f5R149



closes odoo/odoo#69383

Signed-off-by: default avatarGéry Debongnie (ged) <ged@openerp.com>
parent 98179a08
No related branches found
No related tags found
No related merge requests found
......@@ -327,9 +327,7 @@ var CrossTabBus = Longpolling.extend({
}
// update channels
else if (key === this._generateKey('channels')) {
var channels = value;
_.each(_.difference(this._channels, channels), this.deleteChannel.bind(this));
_.each(_.difference(channels, this._channels), this.addChannel.bind(this));
this._channels = value;
}
// update options
else if (key === this._generateKey('options')) {
......
......@@ -2,6 +2,7 @@ odoo.define('web.bus_tests', function (require) {
"use strict";
var BusService = require('bus.BusService');
var CrossTabBus = require('bus.CrossTab');
var AbstractStorageService = require('web.AbstractStorageService');
var RamStorage = require('web.RamStorage');
var testUtils = require('web.test_utils');
......@@ -316,4 +317,81 @@ QUnit.module('Bus', {
}, 3);
});
});});
QUnit.test('two tabs calling addChannel simultaneously', function (assert) {
assert.expect(5);
var id = 1;
testUtils.patch(CrossTabBus, {
init: function () {
this._super.apply(this, arguments);
this.__tabId__ = id++;
},
addChannel: function (channel) {
assert.step('Tab ' + this.__tabId__ + ': addChannel ' + channel);
this._super.apply(this, arguments);
},
deleteChannel: function (channel) {
assert.step('Tab ' + this.__tabId__ + ': deleteChannel ' + channel);
this._super.apply(this, arguments);
},
});
var pollDeferred;
var parentTab1 = new Widget();
testUtils.addMockEnvironment(parentTab1, {
data: {},
services: {
local_storage: LocalStorageServiceMock,
},
mockRPC: function (route) {
if (route === '/longpolling/poll') {
pollDeferred = $.Deferred();
pollDeferred.abort = (function () {
this.reject({message: "XmlHttpRequestError abort"}, $.Event());
}).bind(pollDeferred);
return pollDeferred;
}
return this._super.apply(this, arguments);
}
});
var parentTab2 = new Widget();
testUtils.addMockEnvironment(parentTab2, {
data: {},
services: {
local_storage: LocalStorageServiceMock,
},
mockRPC: function (route) {
if (route === '/longpolling/poll') {
pollDeferred = $.Deferred();
pollDeferred.abort = (function () {
this.reject({message: "XmlHttpRequestError abort"}, $.Event());
}).bind(pollDeferred);
return pollDeferred;
}
return this._super.apply(this, arguments);
}
});
var tab1 = new CrossTabBus(parentTab1);
var tab2 = new CrossTabBus(parentTab2);
tab1.addChannel("alpha");
tab2.addChannel("alpha");
tab1.addChannel("beta");
tab2.addChannel("beta");
assert.verifySteps([
"Tab 1: addChannel alpha",
"Tab 2: addChannel alpha",
"Tab 1: addChannel beta",
"Tab 2: addChannel beta",
]);
testUtils.unpatch(CrossTabBus);
parentTab1.destroy();
parentTab2.destroy();
});
});
});
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment