ClassCastException after plugin gets reloaded #19
Comments
|
The problem is caused by storing instances of a class provided by this plugin in a cache. Whenever such an instance gets stored in a cache, its class is being references by that cache. When the plugin gets reloaded/updated, the PluginClassLoader that loaded the class gets replaced. When the newly loaded plugin tries to interact with the cache (that still has a reference to 'old' data), instances from the same class, but loaded by both the old as well as new PluginClassLoader, are being used, causing class cast exceptions. Note that I believe this problem to apply to all Openfire plugins that store instances of classes that they provide in a cache. I don't think that this problem is specific to Hazelcast either: it also applies to caches in a non-clustered setup. A workaround for this problem is: restart Openfire whenever the plugin gets updated/reloaded. |
|
The only reliable way to prevent this issue that I can think of is to prevent using classes provided by anything but the Openfire classloader. In other words, do not add instances of classes provided by the plugin into a cache. |
|
@GregDThomas what's your take on this? |
…updating the plugin. This commit replaces the usage of classes provided by the plugin itself during cache usage. ClassCastExceptions can occur when a cache has a reference to a class that was loaded by a PluginClassLoader from a previous incarnation of this plugin. See igniterealtime#19 for more details.
That behaviour does seem odd, but certainly explains this problem (and possibly others). I've got around similar problems by caching a String - the JSON representation of the object I wanted to cache, using Jackson (https://www.baeldung.com/jackson-object-mapper-tutorial if you're not already familiar with it). Looking at your fix, you've got two caches, one for each field. That's probably fine, but clearly becomes more of a problem if the number of fields grows. I'd be very tempted to start with a JSON cache now, tbh. |
|
(on reflection, I'd be very tempted to write a couple of default methods to the Cache class, |
|
Why JSON, and not, for example, XML - which is a lot closer to XMPP, and possibly more type-safe? It might even we worth considering using Java native / object serialization, for complete type safety. All classes for which instances are stored in a cache must be Serializable anyway. We might loose 'human readability' (which might be restored by having a smart utility method, but is limited to the admin console anyway), but it'll be a lot more efficient. I was also thinking that it might be a good idea to somehow log a warning, when a Cache is instantiated or used with instances of classes that are loaded by a PluginClassLoader. That might help identify potential affected code. |
|
Why JSON? At the time I was doing it, I was working in JSON a lot (carried over XMPP!) But yes - XML or standard serialization would also do it. Though a textual format may help with the display on the admin screen. And yes, the warning sounds like a good idea. |
|
Although I think you're right in that we can improve on this significantly, I'm going to merge my fix that simply prevents using any plugin-provided class for now. It's cumbersome and adds code complexity, but works. We should replace this when we've got the infrastructure in place that will 'automagically' does some kind of serialization (always? only for plugins?), but for now, I'd like to release something that no longer has the problem. |
|
I believe that I have provided a suitable implementation in igniterealtime/Openfire#1926 @GregDThomas, your suggested default implementation (quoted below) didn't work, as the objectmapped keys and values are Strings (obviously). That does not match with the generics definition of the Cache (K and V). Instead, I've opted for a model that uses delegation to do much of the same.
|
|
That's fine; the above code was written off the top of my head, the implementation itself looks good. |
When the plugin gets reloaded (or reinstalled, upgraded), then ClassCastExceptions are very likely to occur (if the plugin has seen usage).
Exceptions relate to the class that's used to store instances of in a cache, as shown below
The text was updated successfully, but these errors were encountered: