Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[JENKINS-72111] Allow Lifecycle to load implementations from plugins #8589

Merged
merged 1 commit into from
Oct 13, 2023

Conversation

jglick
Copy link
Member

@jglick jglick commented Oct 10, 2023

See JENKINS-72111 and #8555 (comment). Unlike the original PR, this requires no API change or plugin initializer, just the existing system property—which had long used UberClassLoader but did not work for a class defined in a plugin.

Testing done

The functional test added here, as well as validation that this works for a Lifecycle implementation in a plugin in CloudBees CI when the system property is set. (Without this patch, setting the system property to the plugin-defined class causes a boot failure, since the class is not loadable when Lifecycle.get is first called early during startup.)

Proposed changelog entries

  • Allow plugins to define a custom Lifecycle.

Desired reviewers

@Vlatombe @jtnord

Before the changes are marked as ready-for-merge:

Maintainer checklist

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I retained sources in the plugin archive, but for the convenience of reviewers:

package test.custom_lifecycle;
import hudson.lifecycle.Lifecycle;
public final class CustomLifecycle extends Lifecycle {
    public int count;
    @Override
    public void restart() {
        count++;
    }
}

activePlugins.add(p);
((UberClassLoader) uberClassLoader).clearCacheMisses();
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I found that UberClassLoader.loaded cached class loading misses after plugins were loaded during startup. This would not normally be noticeable, but in this case a call to Lifecycle.get before initializing plugins would cache the miss and it would not subsequently be cleared.

Comment on lines -937 to +939
((UberClassLoader) uberClassLoader).loaded.clear();
((UberClassLoader) uberClassLoader).clearCacheMisses();
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is for dynamic plugin loading, so already cleared the cache, though also cache hits which seemed unnecessary.

@Vlatombe Vlatombe changed the title Allow Lifecycle to load implementations from plugins [JENKINS-72111] Allow Lifecycle to load implementations from plugins Oct 10, 2023
@timja timja added the developer Changes which impact plugin developers label Oct 11, 2023
@timja
Copy link
Member

timja commented Oct 11, 2023

/label ready-for-merge


This PR is now ready for merge, after ~24 hours, we will merge it if there's no negative feedback.

Thanks!

@comment-ops-bot comment-ops-bot bot added the ready-for-merge The PR is ready to go, and it will be merged soon if there is no negative feedback label Oct 11, 2023
@NotMyFault NotMyFault merged commit 298e34b into jenkinsci:master Oct 13, 2023
16 checks passed
@jglick jglick deleted the Lifecycle branch October 13, 2023 16:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
developer Changes which impact plugin developers ready-for-merge The PR is ready to go, and it will be merged soon if there is no negative feedback
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants