Skip to content
Permalink
Browse files

[FIXED JENKINS-23098]

Reference: ZD-19531

Looking at [4], one notices that three threads are in an effective dead lock state around synchronizeOnMark. I extracted relevant part into [5].

Thread #1661 is trying to report a discovered mark, but blocking [1]. Thread #1665 is inside synchronizeOnMark, on markCountLock.wait() [2]. Thread #1667 is stuck on Future.get() and hasn't returned [3], which holds the lock that blocks [1] from unblocking [2].

The root problem is that synchronizeOnMark method is never meant to be concurrently executed. But given the way the lock is used, if one thread gets to wait(), it's possible that another thread would come along and go into this function.

In this change, I'm preventing that by introducing another lock to serialize the execution of the entire synchronizeOnMark() call. I'm not using the "this" object for locking because it's already used for another purpose (see the lock() method)

I'm not yet clear on why the synchronizeOnMark() method is called concurrently to begin with. The interaction with the -T option of Maven is suspected.

[1] https://gist.github.com/kohsuke/374c22e737a77c9b0421#file-gistfile1-txt-L2
[2] https://gist.github.com/kohsuke/374c22e737a77c9b0421#file-gistfile1-txt-L34
[3] https://gist.github.com/kohsuke/374c22e737a77c9b0421#file-gistfile1-txt-L71
[4] https://gist.github.com/abayer/7ff4de807c6373eec40d
[5] https://gist.github.com/kohsuke/374c22e737a77c9b0421
  • Loading branch information...
kohsuke committed Jul 9, 2014
1 parent 11cfd5e commit b145d5925ddeae2d697743920da204e6991375ac
Showing with 22 additions and 15 deletions.
  1. +22 −15 src/main/java/hudson/maven/SplittableBuildListener.java
@@ -79,6 +79,7 @@

private int markCount = 0;
private final Object markCountLock = new Object();
private final Object synchronizeLock = new Object();

public SplittableBuildListener(BuildListener core) {
this.core = core;
@@ -141,21 +142,27 @@ protected void onMarkFound() {
* we will not receive any extra bytes after the marker string.
*/
public void synchronizeOnMark(Channel ch) throws IOException, InterruptedException {
synchronized (markCountLock) {
int start = markCount;

// have the remote send us a mark
Future<Void> f = ch.callAsync(new SendMark());

// and block until we receive a mark
while (markCount==start && !f.isDone())
markCountLock.wait(10*1000);

// if SendMark fails, then we fail
try {
f.get();
} catch (ExecutionException e) {
throw new IOException2(e);
// this lock ensures that multiple concurrent executions of synchronizeOnMark get serialized
// and happens one at a time
synchronized (synchronizeLock) {

// this lock is for wait/notify idiom
synchronized (markCountLock) {
int start = markCount;

// have the remote send us a mark
Future<Void> f = ch.callAsync(new SendMark());

// and block until we receive a mark
while (markCount == start && !f.isDone())
markCountLock.wait(10 * 1000);

// if SendMark fails, then we fail
try {
f.get();
} catch (ExecutionException e) {
throw new IOException2(e);
}
}
}
}

0 comments on commit b145d59

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