[WARN ] [tomation.script.ui.HalfBathMotionOff] - Do not use the shared cache to store the object with key 'MasterBathLightTimer', as it is not thread-safe and can cause script execution failures. Only store primitives in the shared cache!
Okay, the timer is an object, so don’t put it in the shared cache (which is awkward but I can work around it). But the doc shared cache section says:
“The access to every key is tracked and the key is removed when all scripts that ever accessed that key are unloaded. If that key stored a timer, the timer will be cancelled. You can use it to store only primitives , as storing objects is not thread-safe and can cause script execution failures.”
Why does it say ‘If that key stored a timer…’ if that is not possible? Should we remove the sentence regarding a timer entirely? And maybe add a sentence that timers are objects that cannot be in a shared cache?
I suspect that the warning and the changes to the docs were made without remembering that sharing Timers between rules was the original reason why the cache was created. It was more expedient to just add a warning than to add locks to the cache bnut no one (including myself) remembered this use case.
@florian-h05 and @J-N-K, do we need to revisit this issue (link above) or, per the comment:
IMO that’s wrong usage of the cache. You should store unmodifiable objects there, and instead of modifying an existing object create a modified copy and put that to the cache.
change the docs and eliminate the possibility to share timers between scripts (which would be unfortunate as the ability to manage the same timer from multuple rules is a relatively common use case) and was the original reason why I files the issue that lead to the creation of the cache in the first place?
On a related note, is there anyway to not have the warning if the shared Object is only being used by one rule? If I have a Script Condition and a Script Action that shares an Object through the cache, it will never be possible to have the multithreaded exception so the warning isn’t necessary.
Note, I’m assuming that setInterval and setTimer creates a JS timer that only exists in the context of that Script. Consequently putting the integer that each returns that is the handle of to the created Timer wouldn’t work because that integer refers to a timer in another context.
I have just done a bit more testing, and it is possible to share objects through the shared cache — as long as they are plain Java and no JS objects.
It is possible to access a timer in shared cache multi-threaded, the same applies to a Java HashMap. When sharing a JS array with the same scripts, you will receive the well-known exception.
So the issue is not as bad as we thought, I will adjust the library code and docs to mention that JS objects shouldn‘t be shared, but timers and other Java objects can be shared without a problem.
Would it be possible to detect when this occurs and use the utility functions in openhab-js to convert between the JS and Java for those? I’m not sure it’s a good idea but it comes to mind as I’ve used those utility functions in my own code at times so I can work in JS arrays in cases where I got a Java List.
It’s probably not the best idea though becuase what if the Array is full of JS Objects? Probably going to have a bad day in that case.
But being able to save OH Timers to the cache is really good. That leaves me satisfied.
Detection is easily possible and done by the library to warn about putting JS objects in the shared cache.
The utility functions are somewhat limited to very generic types, and I don’t think it is a good idea to automatically convert. I think if a user needs to put JS objects in shared cache (which I don’t think is regularly needed), he should think about the way to do it with native Java objects.
