Skip to content
Permalink
Browse files

Fix issue JENKINS-17550, make DisplayAllocator singleton so it can ke…

…ep track of allocated displays. Also fixes JENKINS-17280.
  • Loading branch information
levsa committed Apr 10, 2013
1 parent 0e0f867 commit 6bdd6098c1d5f2a5aeb52fd05fa2f82d85782f96
@@ -15,24 +15,20 @@
*/
private final Set<Integer> allocatedNumbers = new HashSet<Integer>();
private final Set<Integer> blacklistedNumbers = new HashSet<Integer>();
private final int minDisplayNumber;
private final int maxDisplayNumber;

public DisplayAllocator(final int minDisplayNumber, final int maxDisplayNumber) {
this.minDisplayNumber = minDisplayNumber;
this.maxDisplayNumber = maxDisplayNumber;
public DisplayAllocator() {
}

private final int getRandomValue() {
return minDisplayNumber + (new Random().nextInt(getRange()));
private final int getRandomValue(final int min, final int max) {
return min + (new Random().nextInt(getRange(min, max)));
}

private int getRange() {
return (maxDisplayNumber + 1) - minDisplayNumber;
private int getRange(final int min, final int max) {
return (max + 1) - min;
}

public synchronized int allocate() {
if (noDisplayNumbersLeft()) {
public synchronized int allocate(final int minDisplayNumber, final int maxDisplayNumber) {
if (noDisplayNumbersLeft(minDisplayNumber, maxDisplayNumber)) {
if (!blacklistedNumbers.isEmpty()) {
blacklistedNumbers.clear();
} else {
@@ -43,7 +39,7 @@ public synchronized int allocate() {
}
int displayNumber;
do {
displayNumber = getRandomValue();
displayNumber = getRandomValue(minDisplayNumber, maxDisplayNumber);
} while(isNotAvailable(displayNumber));
allocatedNumbers.add(displayNumber);
return displayNumber;
@@ -53,8 +49,8 @@ private boolean isNotAvailable(int number) {
return allocatedNumbers.contains(number) || blacklistedNumbers.contains(number);
}

private boolean noDisplayNumbersLeft() {
return allocatedNumbers.size() + blacklistedNumbers.size() >= getRange();
private boolean noDisplayNumbersLeft(final int min, final int max) {
return allocatedNumbers.size() + blacklistedNumbers.size() >= getRange(min, max);
}

public synchronized void free(int n) {
@@ -30,7 +30,7 @@

/**
* {@link BuildWrapper} that runs <tt>xvnc</tt>.
*
*
* @author Kohsuke Kawaguchi
*/
public class Xvnc extends BuildWrapper {
@@ -39,11 +39,6 @@
*/
public boolean takeScreenshot;

/**
* Manages display numbers in use.
*/
private DisplayAllocator allocator;

private static final String FILENAME_SCREENSHOT = "screenshot.jpg";

@DataBoundConstructor
@@ -52,7 +47,7 @@ public Xvnc(boolean takeScreenshot) {
}

@Override
public Environment setUp(AbstractBuild build, final Launcher launcher, BuildListener listener)
public Environment setUp(AbstractBuild build, final Launcher launcher, BuildListener listener)
throws IOException, InterruptedException {
final PrintStream logger = listener.getLogger();
DescriptorImpl DESCRIPTOR = Hudson.getInstance().getDescriptorByType(DescriptorImpl.class);
@@ -62,28 +57,29 @@ public Environment setUp(AbstractBuild build, final Launcher launcher, BuildList
|| build.getBuiltOn().getNodeProperties().get(NodePropertyImpl.class) != null) {
return new Environment(){};
}

if (DESCRIPTOR.skipOnWindows && !launcher.isUnix()) {
return new Environment(){};
}

if (DESCRIPTOR.cleanUp) {
maybeCleanUp(launcher, listener);
}

String cmd = Util.nullify(DESCRIPTOR.xvnc);
allocator = new DisplayAllocator(DESCRIPTOR.minDisplayNumber, DESCRIPTOR.maxDisplayNumber);
if (cmd == null) {
cmd = "vncserver :$DISPLAY_NUMBER -localhost -nolisten tcp";
}

return doSetUp(build, launcher, logger, cmd, 10);
return doSetUp(build, launcher, logger, cmd, 10, DESCRIPTOR.minDisplayNumber,
DESCRIPTOR.maxDisplayNumber);
}

private Environment doSetUp(AbstractBuild build, final Launcher launcher, final PrintStream logger,
String cmd, int retries) throws IOException, InterruptedException {
String cmd, int retries, int minDisplayNumber, int maxDisplayNumber)
throws IOException, InterruptedException {

final int displayNumber = allocator.allocate();
final int displayNumber = allocator.allocate(minDisplayNumber, maxDisplayNumber);
final String actualCmd = Util.replaceMacro(cmd, Collections.singletonMap("DISPLAY_NUMBER",String.valueOf(displayNumber)));

logger.println(Messages.Xvnc_STARTING());
@@ -105,7 +101,8 @@ private Environment doSetUp(AbstractBuild build, final Launcher launcher, final
//allocator.free(displayNumber);
allocator.blacklist(displayNumber);
if (retries > 0) {
return doSetUp(build, launcher, logger, cmd, retries - 1);
return doSetUp(build, launcher, logger, cmd, retries - 1,
minDisplayNumber, maxDisplayNumber);
} else {
throw new IOException(message);
}
@@ -147,11 +144,16 @@ public boolean tearDown(AbstractBuild build, BuildListener listener) throws IOEx
};
}

/**
* Manages display numbers in use.
*/

This comment has been minimized.

Copy link
@olivergondza
private static final DisplayAllocator allocator = new DisplayAllocator();

/**
* Whether {@link #maybeCleanUp} has already been run on a given node.
*/
private static final Map<Node,Boolean> cleanedUpOn = new WeakHashMap<Node,Boolean>();

// XXX I18N
private static synchronized void maybeCleanUp(Launcher launcher, BuildListener listener) throws IOException, InterruptedException {
Node node = Computer.currentComputer().getNode();
@@ -168,10 +170,10 @@ private static synchronized void maybeCleanUp(Launcher launcher, BuildListener l
launcher.launch().stdout(logger).cmds("pkill", "Xrealvnc").join();
launcher.launch().stdout(logger).cmds("sh", "-c", "rm -f /tmp/.X*-lock /tmp/.X11-unix/X*").join();
}

@Extension
public static final class DescriptorImpl extends BuildWrapperDescriptor {

/**
* xvnc command line. This can include macro.
*
@@ -180,20 +182,20 @@ private static synchronized void maybeCleanUp(Launcher launcher, BuildListener l
public String xvnc;

/*
* Base X display number.
* Base X display number.
*/
public int minDisplayNumber = 10;

/*
* Maximum X display number.
* Maximum X display number.
*/
public int maxDisplayNumber = 99;

/**
* If true, skip xvnc launch on all Windows slaves.
*/
public boolean skipOnWindows = true;

/**
* If true, try to clean up old processes and locks when first run.
*/
@@ -8,88 +8,93 @@

import java.util.Arrays;

import org.junit.Before;
import org.junit.Test;

public class DisplayAllocatorTest {

private static final int MAX = 3;
private static final int MIN = 0;
private DisplayAllocator allocator;

@Before
public void setup() {
allocator = new DisplayAllocator();
}

@Test
public void throwsExceptionWhenAllDisplaysTaken() {
DisplayAllocator allocator = new DisplayAllocator(MIN, MAX);
int range = MAX - MIN + 1;
for (int i=0; i<range; i++) {
allocator.allocate();
allocate();
}
try {
allocator.allocate();
allocate();
fail("Expected exception because all displays are allocated, none received");
} catch(RuntimeException e) {
}
}

private int allocate() {
return allocator.allocate(MIN, MAX);
}

@Test
public void doesNotAllocateBlacklistedDisplays() {
DisplayAllocator allocator = new DisplayAllocator(MIN, MAX);
int badDisplay = MIN + 1;
int range = MAX - MIN + 1 - 1; // one bad display
allocator.blacklist(badDisplay);
for (int i=0; i<range; i++) {
assertThat(allocator.allocate(), not(equalTo(badDisplay)));
assertThat(allocate(), not(equalTo(badDisplay)));
}
}

@Test
public void clearsBlacklistWhenAllDisplaysTaken() {
DisplayAllocator allocator = new DisplayAllocator(MIN, MAX);
int badDisplay = MIN + 1;
allocator.blacklist(badDisplay);
int range = MAX - MIN + 1 - 1;
for (int i=0; i<range; i++) {
assertThat(allocator.allocate(), not(equalTo(badDisplay)));
assertThat(allocate(), not(equalTo(badDisplay)));
}
assertThat(allocator.allocate(), equalTo(badDisplay));
assertThat(allocate(), equalTo(badDisplay));
// now all displays are taken
try {
allocator.allocate();
allocate();
fail("Expected exception because all displays are allocated, none received");
} catch(RuntimeException e) {
}
}

@Test
public void freesPortWhenBlacklisted() {
DisplayAllocator allocator = new DisplayAllocator(0, 1);
int first = allocator.allocate();
int second = allocator.allocate();
allocator.allocate(0, 1);
int second = allocator.allocate(0, 1);
allocator.blacklist(second);
assertThat(allocator.allocate(), equalTo(second));
assertThat(allocator.allocate(0, 1), equalTo(second));
}

@Test
public void canHandleWhenAllDisplaysAreBlacklisted() {
DisplayAllocator allocator = new DisplayAllocator(MIN, MAX);
int range = MAX - MIN + 1;
for (int i=0; i<range; i++) {
allocator.allocate();
allocate();
allocator.blacklist(i);
}
try {
allocator.allocate();
allocate();
} catch(RuntimeException e) {
fail("Blacklist should have been cleared.");
}
}

@Test
public void doesReturnAllNumbersInRangeInclusive() {
DisplayAllocator allocator = new DisplayAllocator(MIN, MAX);
int range = MAX - MIN + 1;
int[] displays = new int[range];
int[] expected = new int[range];
for (int i=0; i<range; i++) {
displays[i] = allocator.allocate();
displays[i] = allocate();
expected[i] = i;
}
Arrays.sort(displays);

0 comments on commit 6bdd609

Please sign in to comment.
You can’t perform that action at this time.