Navigation Menu

Skip to content

Commit

Permalink
Don't keep Extension instances
Browse files Browse the repository at this point in the history 
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
rinrinne committed Sep 4, 2014
1 parent 96802e0 commit 500bd4d
Show file tree
Hide file tree
Showing 5 changed files with 88 additions and 71 deletions.
28 changes: 12 additions & 16 deletions .../sonyericsson/hudson/plugins/gerrit/trigger/dependency/DependencyQueueTaskDispatcher.java
View file
Expand Up @@ -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;
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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;
Expand Down
30 changes: 11 additions & 19 deletions ...va/com/sonyericsson/hudson/plugins/gerrit/trigger/gerritnotifier/ToGerritRunListener.java
View file
Expand Up @@ -33,6 +33,7 @@

import hudson.EnvVars;
import hudson.Extension;
import hudson.ExtensionList;
import hudson.FilePath;
import hudson.model.AbstractBuild;
import hudson.model.AbstractProject;
Expand All @@ -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
Expand Down
88 changes: 55 additions & 33 deletions ...main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/hudsontrigger/GerritTrigger.java
View file
Expand Up @@ -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);
Expand Down Expand Up @@ -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());
}
}

Expand All @@ -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());
}
}

Expand All @@ -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);
Expand Down Expand Up @@ -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)) {
Expand Down Expand Up @@ -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.");
Expand Down
7 changes: 5 additions & 2 deletions ...com/sonyericsson/hudson/plugins/gerrit/trigger/hudsontrigger/actions/RetriggerAction.java
View file
Expand Up @@ -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());
}
}

Expand Down
6 changes: 5 additions & 1 deletion .../sonyericsson/hudson/plugins/gerrit/trigger/hudsontrigger/actions/RetriggerAllAction.java
View file
Expand Up @@ -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;
Expand Down

0 comments on commit 500bd4d

Please sign in to comment.