Copilot commented on code in PR #7088:
URL: https://github.com/apache/shenyu/pull/7088#discussion_r4032823985
##########
shenyu-web/src/main/java/org/apache/shenyu/web/loader/ShenyuExtPathPluginJarLoader.java:
##########
@@ -51,12 +51,16 @@ public static synchronized List<PluginJarParser.PluginJar>
loadExtendPlugins(fin
for (File file : jarFiles) {
String absolutePath = file.getAbsolutePath();
currentPaths.add(absolutePath);
- if (pluginJarName.contains(absolutePath)) {
- continue;
- }
byte[] pluginBytes = Files.readAllBytes(Paths.get(absolutePath));
PluginJarParser.PluginJar uploadPluginJar =
PluginJarParser.parseJar(pluginBytes);
uploadPluginJar.setAbsolutePath(absolutePath);
+ if (pluginJarName.contains(absolutePath)) {
+ ShenyuPluginClassLoaderHolder holder =
ShenyuPluginClassLoaderHolder.getSingleton();
+ if (holder.hasPluginClassLoader(absolutePath,
uploadPluginJar.getVersion())) {
+ continue;
+ }
+ holder.removePluginClassLoader(absolutePath);
Review Comment:
A successful reload still leaves any `PluginDataHandler` from the old JAR
active. `ShenyuLoaderService` forwards the replacement handler to
`CommonPluginDataSubscriber.putExtendPluginDataHandler`, but that method uses
`computeIfAbsent`, so an existing plugin name retains the old instance from the
closed class loader. The reload path must replace the subscriber entry (with
appropriate precedence handling) so future plugin/selector/rule updates use the
new handler.
This issue also appears on line 62 of the same file.
##########
shenyu-web/src/main/java/org/apache/shenyu/web/loader/ShenyuExtPathPluginJarLoader.java:
##########
@@ -51,12 +51,16 @@ public static synchronized List<PluginJarParser.PluginJar>
loadExtendPlugins(fin
for (File file : jarFiles) {
String absolutePath = file.getAbsolutePath();
currentPaths.add(absolutePath);
- if (pluginJarName.contains(absolutePath)) {
- continue;
- }
byte[] pluginBytes = Files.readAllBytes(Paths.get(absolutePath));
PluginJarParser.PluginJar uploadPluginJar =
PluginJarParser.parseJar(pluginBytes);
uploadPluginJar.setAbsolutePath(absolutePath);
+ if (pluginJarName.contains(absolutePath)) {
+ ShenyuPluginClassLoaderHolder holder =
ShenyuPluginClassLoaderHolder.getSingleton();
+ if (holder.hasPluginClassLoader(absolutePath,
uploadPluginJar.getVersion())) {
+ continue;
+ }
+ holder.removePluginClassLoader(absolutePath);
Review Comment:
The old loader is closed before the replacement is actually instantiated and
registered. If `loadUploadedJarPlugins()` fails (or returns no results after
its per-class exceptions), `createPluginClassLoader()` has already cached the
new version, so later scans skip it and the previous Spring beans have already
been destroyed. Keep the old loader active until the candidate loads
successfully, then atomically swap it; failed candidates should remain
retryable.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
