Skip to content
Permalink
Browse files
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
  • Loading branch information
rinrinne committed Sep 4, 2014
1 parent 96802e0 commit 500bd4dc496a027bc1feac169fd71c304b4ebe32
@@ -24,6 +24,7 @@
package com.sonyericsson.hudson.plugins.gerrit.trigger.dependency;

import hudson.Extension;
import hudson.ExtensionList;
import hudson.model.AbstractBuild;
import hudson.model.AbstractProject;
import hudson.model.Cause;
@@ -66,12 +67,11 @@
* @author Yannick Bréhon <yannick.brehon@smartmatic.com>
*/
@Extension
public class DependencyQueueTaskDispatcher extends QueueTaskDispatcher
public final class DependencyQueueTaskDispatcher extends QueueTaskDispatcher
implements GerritEventLifecycleListener, GerritEventListener {

private static final Logger logger = LoggerFactory.getLogger(DependencyQueueTaskDispatcher.class);
private Set<GerritTriggeredEvent> currentlyTriggeringEvents;
private static DependencyQueueTaskDispatcher instance;

/**
* Default constructor.
@@ -102,18 +102,12 @@ public DependencyQueueTaskDispatcher() {
* @return the instance.
*/
public static DependencyQueueTaskDispatcher getInstance() {
if (instance == null) {
for (QueueTaskDispatcher listener : all()) {
if (listener instanceof DependencyQueueTaskDispatcher) {
instance = (DependencyQueueTaskDispatcher)listener;
break;
}
}
}
if (instance == null) {
logger.error("DependencyQueueTaskDispatcher Singleton not initialized on time");
ExtensionList<DependencyQueueTaskDispatcher> dispatchers =
Jenkins.getInstance().getExtensionList(DependencyQueueTaskDispatcher.class);
if (dispatchers == null) {
return null;
}
return instance;
return dispatchers.get(0);
}

@Override
@@ -203,9 +197,11 @@ protected List<AbstractProject> getBlockingDependencyProjects(List<AbstractProje
GerritTriggeredEvent event) {
List<AbstractProject> blockingProjects = new ArrayList<AbstractProject>();
ToGerritRunListener toGerritRunListener = ToGerritRunListener.getInstance();
for (AbstractProject dependency : dependencies) {
if (toGerritRunListener.isProjectTriggeredAndIncomplete(dependency, event)) {
blockingProjects.add(dependency);
if (toGerritRunListener != null) {
for (AbstractProject dependency : dependencies) {
if (toGerritRunListener.isProjectTriggeredAndIncomplete(dependency, event)) {
blockingProjects.add(dependency);
}
}
}
return blockingProjects;
@@ -33,6 +33,7 @@

import hudson.EnvVars;
import hudson.Extension;
import hudson.ExtensionList;
import hudson.FilePath;
import hudson.model.AbstractBuild;
import hudson.model.AbstractProject;
@@ -41,51 +42,42 @@
import hudson.model.Result;
import hudson.model.TaskListener;
import hudson.model.listeners.RunListener;

import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import java.io.IOException;
import java.util.List;

import jenkins.model.Jenkins;

/**
* The Big RunListener in charge of coordinating build results and reporting back to Gerrit.
*
* @author Robert Sandell &lt;robert.sandell@sonyericsson.com&gt;
*/
@Extension(ordinal = ToGerritRunListener.ORDINAL)
public class ToGerritRunListener extends RunListener<AbstractBuild> {
public final class ToGerritRunListener extends RunListener<AbstractBuild> {

/**
* The ordering of this extension.
*/
public static final int ORDINAL = 10003;
private static final Logger logger = LoggerFactory.getLogger(ToGerritRunListener.class);
private static ToGerritRunListener instance;
private transient BuildMemory memory;

/**
* Default Constructor.
*/
public ToGerritRunListener() {
super(AbstractBuild.class);
memory = new BuildMemory();
}
private final transient BuildMemory memory = new BuildMemory();

/**
* Returns the registered instance of this class from the list of all listeners.
*
* @return the instance.
*/
public static ToGerritRunListener getInstance() {
if (instance == null) {
for (RunListener listener : all()) {
if (listener instanceof ToGerritRunListener) {
instance = (ToGerritRunListener)listener;
break;
}
}
ExtensionList<ToGerritRunListener> listeners =
Jenkins.getInstance().getExtensionList(ToGerritRunListener.class);
if (listeners == null) {
return null;
}
return instance;
return listeners.get(0);
}

@Override
@@ -464,7 +464,10 @@ public void gerritEvent(ManualPatchsetCreated event) {
*/
private void notifyOnTriggered(GerritTriggeredEvent event) {
if (!silentMode) {
ToGerritRunListener.getInstance().onTriggered(myProject, event);
ToGerritRunListener listener = ToGerritRunListener.getInstance();
if (listener != null) {
listener.onTriggered(myProject, event);
}
} else {
if (event instanceof GerritEventLifecycle) {
((GerritEventLifecycle)event).fireProjectTriggered(myProject);
@@ -737,25 +740,30 @@ private List<ParameterValue> getDefaultParametersValues(AbstractProject project)
* @param context the previous context.
*/
public void retriggerThisBuild(TriggerContext context) {
if (context.getThisBuild().getProject().isBuildable()
&& !ToGerritRunListener.getInstance().isBuilding(context.getThisBuild().getProject(),
if (context.getThisBuild().getProject().isBuildable()) {
ToGerritRunListener listener = ToGerritRunListener.getInstance();
if (listener != null) {
if (!listener.isBuilding(context.getThisBuild().getProject(),
context.getEvent())) {

Provider provider = initializeProvider(context.getEvent());
Provider provider = initializeProvider(context.getEvent());

// If serverName in event no longer exists, server may have been renamed/removed, so use current serverName
if (!isAnyServer() && !PluginImpl.getInstance().containsServer(provider.getName())) {
provider.setName(serverName);
}
// If serverName in event no longer exists, server may have been renamed/removed,
// so use current serverName
if (!isAnyServer() && !PluginImpl.getInstance().containsServer(provider.getName())) {
provider.setName(serverName);
}

if (!silentMode) {
ToGerritRunListener.getInstance().onRetriggered(
context.getThisBuild().getProject(),
context.getEvent(),
context.getOtherBuilds());
if (!silentMode) {
listener.onRetriggered(
context.getThisBuild().getProject(),
context.getEvent(),
context.getOtherBuilds());
}
final GerritUserCause cause = new GerritUserCause(context.getEvent(), silentMode);
schedule(cause, context.getEvent(), context.getThisBuild().getProject());
}
}
final GerritUserCause cause = new GerritUserCause(context.getEvent(), silentMode);
schedule(cause, context.getEvent(), context.getThisBuild().getProject());
}
}

@@ -770,16 +778,21 @@ public void retriggerThisBuild(TriggerContext context) {
*/
public void retriggerAllBuilds(TriggerContext context) {
DependencyQueueTaskDispatcher dependencyQueueTaskDispatcher = DependencyQueueTaskDispatcher.getInstance();
if (!ToGerritRunListener.getInstance().isBuilding(context.getEvent())) {
dependencyQueueTaskDispatcher.onTriggeringAll(context.getEvent());
retrigger(context.getThisBuild().getProject(), context.getEvent());
for (AbstractBuild build : context.getOtherBuilds()) {
GerritTrigger trigger = (GerritTrigger)build.getProject().getTrigger(GerritTrigger.class);
if (trigger != null) {
trigger.retrigger(build.getProject(), context.getEvent());
if (dependencyQueueTaskDispatcher != null) {
ToGerritRunListener listener = ToGerritRunListener.getInstance();
if (listener != null) {
if (!listener.isBuilding(context.getEvent())) {
dependencyQueueTaskDispatcher.onTriggeringAll(context.getEvent());
retrigger(context.getThisBuild().getProject(), context.getEvent());
for (AbstractBuild build : context.getOtherBuilds()) {
GerritTrigger trigger = (GerritTrigger)build.getProject().getTrigger(GerritTrigger.class);
if (trigger != null) {
trigger.retrigger(build.getProject(), context.getEvent());
}
}
dependencyQueueTaskDispatcher.onDoneTriggeringAll(context.getEvent());
}
}
dependencyQueueTaskDispatcher.onDoneTriggeringAll(context.getEvent());
}
}

@@ -794,7 +807,10 @@ private void retrigger(AbstractProject project, GerritTriggeredEvent event) {
if (project.isBuildable()) {
initializeProvider(event);
if (!silentMode) {
ToGerritRunListener.getInstance().onRetriggered(project, event, null);
ToGerritRunListener listener = ToGerritRunListener.getInstance();
if (listener != null) {
listener.onRetriggered(project, event, null);
}
}
GerritUserCause cause = new GerritUserCause(event, silentMode);
schedule(cause, event, project);
@@ -835,12 +851,15 @@ private boolean isInteresting(GerritTriggeredEvent event) {
return false;
}

if (ToGerritRunListener.getInstance().isProjectTriggeredAndIncomplete(myProject, event)) {
logger.trace("Already triggered and imcompleted.");
return false;
} else if (ToGerritRunListener.getInstance().isTriggered(myProject, event)) {
logger.trace("Already triggered.");
return false;
ToGerritRunListener listener = ToGerritRunListener.getInstance();
if (listener != null) {
if (listener.isProjectTriggeredAndIncomplete(myProject, event)) {
logger.trace("Already triggered and imcompleted.");
return false;
} else if (listener.isTriggered(myProject, event)) {
logger.trace("Already triggered.");
return false;
}
}

if (!shouldTriggerOnEventType(event)) {
@@ -946,9 +965,12 @@ private boolean commentAddedMatch(CommentAdded event) {
*/
public void gerritEvent(CommentAdded event) {
logger.trace("event: {}", event);
if (ToGerritRunListener.getInstance().isBuilding(myProject, event)) {
logger.trace("Already building.");
return;
ToGerritRunListener listener = ToGerritRunListener.getInstance();
if (listener != null) {
if (listener.isBuilding(myProject, event)) {
logger.trace("Already building.");
return;
}
}
if (isInteresting(event) && commentAddedMatch(event)) {
logger.trace("The event is interesting.");
@@ -95,9 +95,12 @@ private boolean isBuilding() {
if (context == null || context.getThisBuild() == null || context.getEvent() == null) {
return false;
} else {
ToGerritRunListener listener = ToGerritRunListener.getInstance();
if (listener == null) {
return context.getThisBuild().getBuild().isBuilding();
}
return context.getThisBuild().getBuild().isBuilding()
|| ToGerritRunListener.getInstance().isBuilding(context.getThisBuild().getProject(),
context.getEvent());
|| listener.isBuilding(context.getThisBuild().getProject(), context.getEvent());
}
}

@@ -108,8 +108,12 @@ private boolean hasOthers() {
*/
private boolean isBuilding() {
if (context != null) {
ToGerritRunListener listener = ToGerritRunListener.getInstance();
if (listener == null) {
return context.getThisBuild().getBuild().isBuilding();
}
return context.getThisBuild().getBuild().isBuilding()
|| ToGerritRunListener.getInstance().isBuilding(context.getEvent());
|| listener.isBuilding(context.getEvent());
} else {
//The correct answer here should be null, but hasPermission takes care of a more "correct" answer.
return false;

0 comments on commit 500bd4d

Please sign in to comment.