Theoretically our Koha::Cache is not as safe as possible, the set_in_cache and get_from_cache subroutines should always clone the structure to avoid uncontrollable behaviors. There are, at least, 2 issues to cover: - get_from_cache does not deep copy on first call - set_in_cache does not deep copy Note that bug 16221 plans to replace Clone::clone with Storable::dclone, it should be built on top of this patch.
Created attachment 50072 [details] [review] Bug 16229: Deep copy on first L2 fetch When a value exists in L2 cache but not in L1 cache, it should be deep copied if needed (i.e. not a scalar). Otherwise the calling code is able to modify the value in cache. Note that is theoretical, it's possible that no code does that.
Created attachment 50073 [details] [review] Bug 16229: Deep copy on setting in cache Koha::Cache->set_in_cache should deep copy (if needed) to avoid the value which has been set in cache to be unintentionally modified later.
Created attachment 50074 [details] [review] Bug 16229: Add the unsafe flag to set_in_cache Could be useful later.
Created attachment 50076 [details] [review] Bug 16229: Add the unsafe flag to set_in_cache Could be useful later.
Created attachment 50199 [details] [review] Bug 16229: Deep copy on first L2 fetch When a value exists in L2 cache but not in L1 cache, it should be deep copied if needed (i.e. not a scalar). Otherwise the calling code is able to modify the value in cache. Note that is theoretical, it's possible that no code does that. Signed-off-by: Jacek Ablewicz <abl@biblos.pk.edu.pl>
Created attachment 50200 [details] [review] Bug 16229: Deep copy on setting in cache Koha::Cache->set_in_cache should deep copy (if needed) to avoid the value which has been set in cache to be unintentionally modified later. Signed-off-by: Jacek Ablewicz <abl@biblos.pk.edu.pl>
Created attachment 50201 [details] [review] Bug 16229: Add the unsafe flag to set_in_cache Could be useful later. Signed-off-by: Jacek Ablewicz <abl@biblos.pk.edu.pl>
Created attachment 50235 [details] [review] Bug 16229: Deep copy on first L2 fetch When a value exists in L2 cache but not in L1 cache, it should be deep copied if needed (i.e. not a scalar). Otherwise the calling code is able to modify the value in cache. Note that is theoretical, it's possible that no code does that. Signed-off-by: Jacek Ablewicz <abl@biblos.pk.edu.pl> Signed-off-by: Tomas Cohen Arazi <tomascohen@unc.edu.ar>
Created attachment 50236 [details] [review] Bug 16229: Deep copy on setting in cache Koha::Cache->set_in_cache should deep copy (if needed) to avoid the value which has been set in cache to be unintentionally modified later. Signed-off-by: Jacek Ablewicz <abl@biblos.pk.edu.pl> Signed-off-by: Tomas Cohen Arazi <tomascohen@unc.edu.ar>
Created attachment 50237 [details] [review] Bug 16229: Add the unsafe flag to set_in_cache Could be useful later. Signed-off-by: Jacek Ablewicz <abl@biblos.pk.edu.pl> Signed-off-by: Tomas Cohen Arazi <tomascohen@unc.edu.ar>
Created attachment 50238 [details] [review] Bug 16229: (QA) POD fixes Signed-off-by: Tomas Cohen Arazi <tomascohen@unc.edu.ar>
Pushed to Master - Should be in the May 2016 Release. Thanks!
Not needed in stable versions, status changed to RESOLVED
(In reply to Julian Maurice from comment #13) > Not needed in stable versions, status changed to RESOLVED Finally needed, because bug 16044 was pushed into 3.22.x Patches pushed to 3.22.x, will be in 3.22.8