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

Don't keep Extension instances #175

Merged
merged 4 commits into from Sep 4, 2014

Conversation

Projects
None yet
3 participants
@rinrinne
Copy link
Member

commented Sep 4, 2014

All instance of Extension extended class must be fully managed by
Jenkins instance. So those must not be reused by other instances.

This patch fixes such issue. Targets are:

ToGerritRunListener
DependencyQueueTaskDispatcher
In addition, the above class is changed to final class. Because getInstance() has possibility to return instance of extended class if those are extendable.

Fix for JENKINS-24575

rinrinne added some commits Sep 4, 2014

Don't keep Extension instances
All instance of Extension extended class must be fully managed by
Jenkins instance. So those must not be reused by other instances.

This patch fixes such issue. Targets are:

* ToGerritRunListener
* DependencyQueueTaskDispatcher

Fix for JENKINS-24575

Task-Url: https://issues.jenkins-ci.org/browse/JENKINS-24575
Fix checkstyle error
This patch increase the threshold of file length.
Permanent fix is to be refactored, but it is out of scope for this
change.
@cloudbees-pull-request-builder

This comment has been minimized.

Copy link

commented Sep 4, 2014

plugins » gerrit-trigger-plugin #326 SUCCESS
This pull request looks good

logger.error("DependencyQueueTaskDispatcher Singleton not initialized on time");
ExtensionList<DependencyQueueTaskDispatcher> dispatchers =
Jenkins.getInstance().getExtensionList(DependencyQueueTaskDispatcher.class);
if (dispatchers == null) {

This comment has been minimized.

Copy link
@rsandell

rsandell Sep 4, 2014

Member

I guess the list could be empty as well causing an indexoutofbounds on line 110. so adding " || dispatchers.isEmpty()" would make it safer.

This comment has been minimized.

Copy link
@rinrinne

rinrinne Sep 4, 2014

Author Member

OK

}
return instance;
return listeners.get(0);

This comment has been minimized.

Copy link
@rsandell

rsandell Sep 4, 2014

Member

same here

@cloudbees-pull-request-builder

This comment has been minimized.

Copy link

commented Sep 4, 2014

plugins » gerrit-trigger-plugin #327 SUCCESS
This pull request looks good

rsandell added a commit that referenced this pull request Sep 4, 2014

@rsandell rsandell merged commit c6ba3f7 into jenkinsci:master Sep 4, 2014

@rinrinne rinrinne deleted the rinrinne:extension-instances branch Nov 4, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.