From 5e4fb93e8da780ae81bce2bb3b66b4fbfad0aafc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=A9dric=20Krier?= Date: Mon, 12 Feb 2024 18:27:26 +0100 Subject: [PATCH] Make icon factory thread-safe By using a single immutable dictionary as source for icon ids, we ensure the coherence during the execution even if other calls are executed in parallel. Closes #13033 --- sao/src/common.js | 157 ++++++++++++++------------------- sao/src/sao.js | 1 - tryton/tryton/common/common.py | 77 +++++++--------- 3 files changed, 101 insertions(+), 134 deletions(-) diff --git a/sao/src/common.js b/sao/src/common.js index 30dc77e631..5ca773a609 100644 --- a/sao/src/common.js +++ b/sao/src/common.js @@ -3083,91 +3083,82 @@ Sao.common.IconFactory = Sao.class_(Object, { batchnum: 10, - name2id: {}, - loaded_icons: {}, - tryton_icons: [], - register_prm: jQuery.when(), - load_icons: function(refresh) { - refresh = refresh || false; + _name2id: {}, + _icons: {}, + load_icons: function(refresh=false) { + const icon_model = new Sao.Model('ir.ui.icon'); + var icons; + try { + icons = icon_model.execute('list_icons', [], {}, false); + } catch (e) { + icons = []; + } + const name2id = {}; + for (const icon of icons) { + name2id[icon[1]] = icon[0]; + } + this._name2id = name2id; if (!refresh) { - for (var icon_name in this.loaded_icons) { - window.URL.revokeObjectURL(this.loaded_icons[icon_name]); + for (const icon_name in this._icons) { + window.URL.revokeObjectURL(this._icons[icon_name]); } + this._icons = {}; } - - var icon_model = new Sao.Model('ir.ui.icon'); - return icon_model.execute('list_icons', [], {}) - .then(icons => { - if (!refresh) { - this.name2id = {}; - this.loaded_icons = {}; - } - this.tryton_icons = []; - - var icon_id, icon_name; - for (var i=0, len=icons.length; i < len; i++) { - icon_id = icons[i][0]; - icon_name = icons[i][1]; - if (refresh && (icon_name in this.loaded_icons)) { - continue; - } - this.tryton_icons.push([icon_id, icon_name]); - this.name2id[icon_name] = icon_id; - } - }); + return name2id; }, - register_icon: function(icon_name) { - if (!icon_name) { - return jQuery.when(); - } else if ((icon_name in this.loaded_icons) || - ~Sao.common.LOCAL_ICONS.indexOf(icon_name)) { - return jQuery.when(); + _get_icon: function(icon_name) { + var url = this._icons[icon_name]; + if (url !== undefined) { + return jQuery.when(url); } - if (this.register_prm.state() == 'pending') { - return this.register_prm.then( - () => this.register_icon(icon_name)); + if (~Sao.common.LOCAL_ICONS.indexOf(icon_name)) { + return jQuery.get('images/' + icon_name + '.svg', null, null, 'text') + .then(icon => { + var img_url = this._convert(icon); + this._icons[icon_name] = img_url; + return img_url; + }) + .fail(() => { + Sao.Logger.error("Unknown icon %s", icon_name); + this._icons[icon_name] = null; + }); } - var loaded_prm; - if (!(icon_name in this.name2id)) { - loaded_prm = this.load_icons(true); - } else { - loaded_prm = jQuery.when(); + var name2id = this._name2id; + if (!(icon_name in name2id)) { + name2id = this.load_icons(true); + if (!(name in name2id)) { + Sao.Logger.error("Unknown icon %s", icon_name); + this._icons[icon_name] = null; + return jQuery.when(); + } } + var ids = []; + for (const name in name2id) { + if ((!(name in this._icons)) || (name == icon_name)) { + ids.push(name2id[name]); + } + } + const idx = ids.indexOf(name2id[icon_name]); + const from = Math.max(Math.round(idx - this.batchnum / 2), 0); + const to = Math.round(idx + this.batchnum / 2); + ids = ids.slice(from, to); var icon_model = new Sao.Model('ir.ui.icon'); - this.register_prm = loaded_prm.then(() => { - const find_array = array => { - var idx, l; - for (idx=0, l=this.tryton_icons.length; idx < l; idx++) { - var icon = this.tryton_icons[idx]; - if (Sao.common.compare(icon, array)) { - break; - } - } - return idx; - }; - var idx = find_array([this.name2id[icon_name], icon_name]); - var from = Math.round(idx - this.batchnum / 2); - from = (from < 0) ? 0 : from; - var to = Math.round(idx + this.batchnum / 2); - var ids = []; - for (const e of this.tryton_icons.slice(from, to)) { - ids.push(e[0]); + var icons; + try { + icons = icon_model.execute( + 'read', [ids, ['name', 'icon']], {}, false); + } catch(e) { + icons = []; + } + for (const icon of icons) { + const icon_url = this._convert(icon.icon); + this._icons[icon.name] = icon_url; + if (icon.name == icon_name) { + url = icon_url; } - - var read_prm = icon_model.execute('read', - [ids, ['name', 'icon']], {}); - return read_prm.then(icons => { - for (const icon of icons) { - var img_url = this._convert(icon.icon); - this.loaded_icons[icon.name] = img_url; - delete this.name2id[icon.name]; - this.tryton_icons.splice( - find_array([icon.id, icon.name]), 1); - } - }); - }); - return this.register_prm; + } + return jQuery.when(url); }, _convert: function(data) { var xml = jQuery.parseXML(data); @@ -3181,21 +3172,7 @@ if (!icon_name) { return jQuery.when(''); } - return this.register_icon(icon_name).then(() => { - if (icon_name in this.loaded_icons) { - return this.loaded_icons[icon_name]; - } else { - return jQuery.get('images/' + icon_name + '.svg', null, null, 'text') - .then(icon => { - var img_url = this._convert(icon); - this.loaded_icons[icon_name] = img_url; - return img_url; - }) - .fail(() => { - Sao.Logger.error("Unknown icon %s", icon_name); - }); - } - }); + return this._get_icon(icon_name); }, get_icon_img: function(icon_name, attrs) { attrs = attrs || {}; diff --git a/sao/src/sao.js b/sao/src/sao.js index d59305e0ae..74429b3f24 100644 --- a/sao/src/sao.js +++ b/sao/src/sao.js @@ -414,7 +414,6 @@ var Sao = { }, session).then(function(preferences) { var deferreds = []; deferreds.push(Sao.common.MODELACCESS.load_models()); - deferreds.push(Sao.common.ICONFACTORY.load_icons()); deferreds.push(Sao.common.MODELHISTORY.load_history()); deferreds.push(Sao.common.MODELNOTIFICATION.load_names()); deferreds.push(Sao.common.VIEW_SEARCH.load_searches()); diff --git a/tryton/tryton/common/common.py b/tryton/tryton/common/common.py index 6758f30057..481df9602f 100644 --- a/tryton/tryton/common/common.py +++ b/tryton/tryton/common/common.py @@ -11,7 +11,7 @@ import re import subprocess import unicodedata import xml.etree.ElementTree as ET -from collections import defaultdict +from collections import OrderedDict, defaultdict from decimal import Decimal from http.server import BaseHTTPRequestHandler, HTTPServer from pathlib import PurePath @@ -60,8 +60,7 @@ logger = logging.getLogger(__name__) class IconFactory: batchnum = 10 - _tryton_icons = [] - _name2id = {} + _name2id = OrderedDict() _icons = {} _local_icons = {} _pixbufs = defaultdict(dict) @@ -75,69 +74,61 @@ class IconFactory: @classmethod def load_icons(cls, refresh=False): - if not refresh: - cls._name2id.clear() - cls._icons.clear() - del cls._tryton_icons[:] - try: icons = rpc.execute('model', 'ir.ui.icon', 'list_icons', rpc.CONTEXT) except TrytonServerError: icons = [] - for icon_id, icon_name in icons: - if refresh and icon_name in cls._icons: - continue - cls._tryton_icons.append((icon_id, icon_name)) - cls._name2id[icon_name] = icon_id + cls._name2id = name2id = OrderedDict((n, i) for i, n in icons) + if not refresh: + cls._icons.clear() + return name2id @classmethod - def register_icon(cls, iconname): - # iconname might be '' when page do not define icon - if (not iconname - or iconname in cls._icons - or iconname in cls._local_icons): - return - if iconname not in cls._name2id: - cls.load_icons(refresh=True) - try: - icon_ref = (cls._name2id[iconname], iconname) - except KeyError: - logger.error(f"Unknown icon {iconname}") - cls._icons[iconname] = None - return - idx = cls._tryton_icons.index(icon_ref) - to_load = slice(max(0, idx - cls.batchnum // 2), + def _get_icon(cls, iconname): + data = cls._icons.get(iconname) + if data is not None: + return data + path = cls._local_icons.get(iconname) + if path is not None: + with open(path, 'rb') as fp: + return fp.read() + + name2id = cls._name2id + if iconname not in name2id: + name2id = cls.load_icons(refresh=True) + if iconname not in name2id: + logger.error(f"Unknown icon {iconname}") + cls._icons[iconname] = None + return + names = [n for n in name2id if n not in cls._icons or n == iconname] + idx = names.index(iconname) + to_load = slice( + max(0, idx - cls.batchnum // 2), idx + cls.batchnum // 2) - ids = [e[0] for e in cls._tryton_icons[to_load]] + ids = [name2id[n] for n in names[to_load]] try: icons = rpc.execute('model', 'ir.ui.icon', 'read', ids, ['name', 'icon'], rpc.CONTEXT) except TrytonServerError: icons = [] + data = None for icon in icons: name = icon['name'] - data = icon['icon'].encode('utf-8') - cls._icons[name] = data - cls._tryton_icons.remove((icon['id'], icon['name'])) - del cls._name2id[icon['name']] + icondata = icon['icon'].encode('utf-8') + cls._icons[name] = icondata + if name == iconname: + data = icondata + return data @classmethod def get_pixbuf(cls, iconname, size=16, color=None, badge=None): if not iconname: return colors = CONFIG['icon.colors'].split(',') - cls.register_icon(iconname) if iconname not in cls._pixbufs[(size, badge)]: - data = None - if iconname in cls._icons: - data = cls._icons[iconname] - elif iconname in cls._local_icons: - path = cls._local_icons[iconname] - with open(path, 'rb') as fp: - data = fp.read() + data = cls._get_icon(iconname) if not data: - logger.error("Unknown icon %s" % iconname) return if not color: color = colors[0]