From 1a3defcb3d1da4a436e346d4f76b6b7a45c20b88 Mon Sep 17 00:00:00 2001 From: Guus der Kinderen Date: Tue, 7 Dec 2021 16:56:33 +0100 Subject: [PATCH] Use plugin classloader when executing task defined in a plugin When a clustered tasks is executed (particularly when that task returns a value), class loading issues can occur when the task is defined in a plugin. The classes that are used by the task might not be accessible by the (context class loader of the) thread that executes the task. When a task is executed that is defined in a plugin, the context class loader of the thread should be switched to the plugin class loader during the execution of the task to work around this issue. Be aware of #74: Using Tasks defined in a plugin causes issues (particularly when reloading that plugin). That problem remains, even with the improvement suggested here. fixes #79 --- changelog.html | 1 + .../util/cache/ClusteredCacheFactory.java | 47 +++++++++++++++---- 2 files changed, 38 insertions(+), 10 deletions(-) diff --git a/changelog.html b/changelog.html index c340899e8..a9d6ff97a 100644 --- a/changelog.html +++ b/changelog.html @@ -50,6 +50,7 @@

  • [Issue #103] - Fix Cluster initialization race condition
  • [Issue #102] - Remove unused code in ClusterListener
  • [Issue #81] - ClusterExternalizableUtil should not ignore provided ClassLoader instances
    • +
  • [Issue #79] - Use plugin classloader when executing task defined in a plugin

    • 3.0.0 -- September 12, 2024

    diff --git a/src/java/org/jivesoftware/openfire/plugin/util/cache/ClusteredCacheFactory.java b/src/java/org/jivesoftware/openfire/plugin/util/cache/ClusteredCacheFactory.java index 3c37294c8..8f2e09866 100644 --- a/src/java/org/jivesoftware/openfire/plugin/util/cache/ClusteredCacheFactory.java +++ b/src/java/org/jivesoftware/openfire/plugin/util/cache/ClusteredCacheFactory.java @@ -420,9 +420,14 @@ public Collection doSynchronousClusterTask(final ClusterTask task, fin final Collection result = new ArrayList<>(); if (!members.isEmpty()) { // Asynchronously execute the task on the other cluster members + logger.debug("Executing MultiTask: " + task.getClass().getName()); + final PluginClassLoader pluginClassLoader = checkForPluginClassLoader(task); + final ClassLoader contextClassLoader = Thread.currentThread().getContextClassLoader(); try { - logger.debug("Executing MultiTask: " + task.getClass().getName()); - checkForPluginClassLoader(task); + // Switch to the classloader that provides the plugin classes, if needed. + if (pluginClassLoader != null) { + Thread.currentThread().setContextClassLoader(pluginClassLoader); + } final Map> futures = hazelcast.getExecutorService(HAZELCAST_EXECUTOR_SERVICE_NAME.getValue()).submitToMembers(new CallableTask<>(task), members); long nanosLeft = TimeUnit.SECONDS.toNanos(MAX_CLUSTER_EXECUTION_TIME.getValue().getSeconds() * members.size()); for (final Future future : futures.values()) { @@ -434,6 +439,11 @@ public Collection doSynchronousClusterTask(final ClusterTask task, fin logger.error("Failed to execute cluster task within " + StringUtils.getFullElapsedTime(MAX_CLUSTER_EXECUTION_TIME.getValue()), te); } catch (final Exception e) { logger.error("Failed to execute cluster task", e); + } finally { + if (pluginClassLoader != null) { + // Revert back to the original classloader. + Thread.currentThread().setContextClassLoader(contextClassLoader); + } } } else { logger.debug("No cluster members selected for cluster task " + task.getClass().getName()); @@ -457,8 +467,13 @@ public T doSynchronousClusterTask(final ClusterTask task, final byte[] no if (member != null) { // Asynchronously execute the task on the target member logger.debug("Executing DistributedTask: " + task.getClass().getName()); - checkForPluginClassLoader(task); + final PluginClassLoader pluginClassLoader = checkForPluginClassLoader(task); + final ClassLoader contextClassLoader = Thread.currentThread().getContextClassLoader(); try { + // Switch to the classloader that provides the plugin classes, if needed. + if (pluginClassLoader != null) { + Thread.currentThread().setContextClassLoader(pluginClassLoader); + } final Future future = hazelcast.getExecutorService(HAZELCAST_EXECUTOR_SERVICE_NAME.getValue()).submitToMember(new CallableTask<>(task), member); result = future.get(MAX_CLUSTER_EXECUTION_TIME.getValue().getSeconds(), TimeUnit.SECONDS); logger.trace("DistributedTask result: {}", result); @@ -466,6 +481,11 @@ public T doSynchronousClusterTask(final ClusterTask task, final byte[] no logger.error("Failed to execute cluster task within " + MAX_CLUSTER_EXECUTION_TIME + " seconds", te); } catch (final Exception e) { logger.error("Failed to execute cluster task", e); + } finally { + if (pluginClassLoader != null) { + // Revert back to the original classloader. + Thread.currentThread().setContextClassLoader(contextClassLoader); + } } } else { final String msg = MessageFormat.format("Requested node {0} not found in cluster", new String(nodeID, StandardCharsets.UTF_8)); @@ -547,11 +567,12 @@ public Lock getLock(final Object key, Cache cache) { * limited by a time interval. * * @param o the instance for which to verify the class loader + * @return The PluginClassLoader that was used to load the instance, if it was loaded by a plugin * @see Issue #74: Warn against usage of plugin-provided classes in Hazelcast */ - protected > void checkForPluginClassLoader(final T o) { - if (o != null && o.getClass().getClassLoader() instanceof PluginClassLoader - && !pluginClassLoaderWarnings.containsKey(o.getClass().getName()) ) + protected > PluginClassLoader checkForPluginClassLoader(final T o) { + PluginClassLoader result = null; + if (o != null && o.getClass().getClassLoader() instanceof PluginClassLoader) { // Try to determine what plugin loaded the offending class. String pluginName = null; @@ -561,17 +582,23 @@ protected > void checkForPluginClassLoader(final T o) { final PluginClassLoader pluginClassloader = XMPPServer.getInstance().getPluginManager().getPluginClassloader(plugin); if (o.getClass().getClassLoader().equals(pluginClassloader)) { pluginName = XMPPServer.getInstance().getPluginManager().getCanonicalName(plugin); + result = pluginClassloader; break; } } } catch (Exception e) { logger.debug("An exception occurred while trying to determine the plugin class loader that loaded an instance of {}", o.getClass(), e); } - logger.warn("An instance of {} that is executed as a cluster task. This will cause issues when reloading " + - "the plugin that provides this class. The plugin implementation should be modified.", - pluginName != null ? o.getClass() + " (provided by plugin " + pluginName + ")" : o.getClass()); - pluginClassLoaderWarnings.put(o.getClass().getName(), Instant.now()); // Note that this Instant is unused. + + // Only print this warning once in a while (a cache entry that is added will eventually be evicted). + if (!pluginClassLoaderWarnings.containsKey(o.getClass().getName())) { + logger.warn("An instance of {} that is executed as a cluster task. This will cause issues when reloading " + + "the plugin that provides this class. The plugin implementation should be modified.", + pluginName != null ? o.getClass() + " (provided by plugin " + pluginName + ")" : o.getClass()); + pluginClassLoaderWarnings.put(o.getClass().getName(), Instant.now()); // Note that this Instant is unused. + } } + return result; } private static class ClusterLock implements Lock {