Opened 8 years ago

Last modified 6 years ago

#4257 closed defect

Freeze cached session objects — at Version 6

Reported by: elexis Owned by:
Priority: Must Have Milestone: Alpha 23
Component: UI & Simulation Keywords:
Cc: Patch:

Description (last modified by elexis)

The functions GetSimState, GetEntityState, GetExtendedEntityState, GetTemplateData, GetTemplateDataWithoutLocalization, GetTechnologyData cache their results in global variables and return these objects each.

However, returning an object means that the reference is returned, not a copy. This isn't an issue if that object was newly created (or cloned). But if the function returns an object which is accessible outside of the scope of the function (f.e. a global cache variable), modifying the returned object modifies the cache, which is not intentional and adulterates the original data.

This is often not considered and has caused a number of bugs already, f.e. r18788 r17324 r17673 r17441 r17712 r17715.

The mentioned functions should use Object.freeze to ensure that the returned object can't be modified.

There are likely many other places in gui/ where global variables are used to cache things, that should be frozen.

Change History (6)

comment:1 by elexis, 8 years ago

Another example are the cached JSON aura and tech templates in the DataTemplateManager. If those were modified somewhere that were pretty bad.

comment:2 by elexis, 8 years ago

Notice Object.freeze is not sufficient as it doesn't work recursively.

comment:3 by javiergodas, 8 years ago

Owner: set to javiergodas
Status: newassigned

comment:4 by elexis, 7 years ago

Resources.js should also freeze the resource objects, once it is included (#3934).

comment:5 by elexis, 7 years ago

Owner: javiergodas removed
Status: assignednew

comment:6 by elexis, 7 years ago

Description: modified (diff)
Milestone: BacklogAlpha 23

Exemplary JS code for recursive freezing (deep-freeze) is described at:

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object/freeze

It should be added to globalscripts/utility.js as it is needed in both simulation and GUI.

Note: See TracTickets for help on using tickets.