Fixed
Status Update
No update yet.
Comments
ap...@google.com
Project: gerrit
Branch: stable-3.8
commit 5e56ebcb3096e9bf18b65996b700a6e0bb264020
Author: Antonio Barone <syntonyze@gmail.com>
Date: Wed Jun 12 16:36:14 2024
Fix reloading of plugins registering dynamic items
When change Ia7ed2a06ec3 refactored the `Plugin` hierarchy by
introducing different plugin specifications, omitted to inherit the
`reloadableHandles` from the parent class and instead, introduced a new
`reloadableHandles` variable in the `JarPlugin` implementation.
This caused the `Plugin.getReloadableHandles()` to always return an
empty list, effectively preventing the reloading of the plugin from
replacing existing reloadable bindings [1] and creating new ones
instead.
Whilst this still allows the `reload` to succeed, it misses the removal
of the registration handle associated with the old plugin instance,
which will then not be de-registered if the new plugin is disabled later
on.
Inherit `reloadableHandles` from the parent `Plugin` class so that
addition and retrieval of reloadable registration handles targets the
same instance variable, allowing the reload to move existing dynamic
registration handles to the new plugin instance, rather than creating a
new one.
[1]https://gerrit.googlesource.com/gerrit/+/refs/heads/stable-3.8/java/com/google/gerrit/server/plugins/PluginGuiceEnvironment.java#305
Bug:
Release-Notes: Fix reload of plugins causing incorrect deregistration of dynamic items
Change-Id: I13896cf1fcabebc26d465aec7c8debc9dddb11be
M java/com/google/gerrit/server/plugins/Plugin.java
M java/com/google/gerrit/server/plugins/ServerPlugin.java
https://gerrit-review.googlesource.com/429703
Branch: stable-3.8
commit 5e56ebcb3096e9bf18b65996b700a6e0bb264020
Author: Antonio Barone <syntonyze@gmail.com>
Date: Wed Jun 12 16:36:14 2024
Fix reloading of plugins registering dynamic items
When change Ia7ed2a06ec3 refactored the `Plugin` hierarchy by
introducing different plugin specifications, omitted to inherit the
`reloadableHandles` from the parent class and instead, introduced a new
`reloadableHandles` variable in the `JarPlugin` implementation.
This caused the `Plugin.getReloadableHandles()` to always return an
empty list, effectively preventing the reloading of the plugin from
replacing existing reloadable bindings [1] and creating new ones
instead.
Whilst this still allows the `reload` to succeed, it misses the removal
of the registration handle associated with the old plugin instance,
which will then not be de-registered if the new plugin is disabled later
on.
Inherit `reloadableHandles` from the parent `Plugin` class so that
addition and retrieval of reloadable registration handles targets the
same instance variable, allowing the reload to move existing dynamic
registration handles to the new plugin instance, rather than creating a
new one.
[1]
Bug:
Release-Notes: Fix reload of plugins causing incorrect deregistration of dynamic items
Change-Id: I13896cf1fcabebc26d465aec7c8debc9dddb11be
M java/com/google/gerrit/server/plugins/Plugin.java
M java/com/google/gerrit/server/plugins/ServerPlugin.java
Description
What steps will reproduce the problem?
DynamicItem. You can build it in-tree by issuing:error_log(or stdout) and notice how it shows the item bounded by the plugin, every 5 seconds:Notice how
ItemBoundByPluginis still correctly bound.Disable the plugin
What is the expected output?
the
ItemBoundByPluginshould be de-registered and the default implementation should be bound instead. In this case, that's theDefaultProjectNameLockManager.What do you see instead?
The
ItemBoundByPluginis still bounded, and it is not de-registered, a.k.a. you can still see in the logs:** Notes **
This seems to be related to an old refactoring of the
Pluginclasses [1], which caused the incorrect duplication of thereloadableRegistrationHandlesinstance.This is preventing the DynamicItem from being correctly hot-swapped to the new plugin during the reload and instead a new one is created, leaving the old DynamicItem bound class around.
[1]https://gerrit-review.googlesource.com/c/gerrit/+/39270