Skip to content

Commit 043a336

Browse files
rpettirpetti
committed
Merge pull request #32 from synopsys-arc-oss/p4-hangs-issue-workaround
"Slave polling hangup" issue workaround (JENKINS-15315)
2 parents 1fc1909 + b8b0115 commit 043a336

File tree

8 files changed

+166
-8
lines changed

8 files changed

+166
-8
lines changed

‎src/main/java/com/tek42/perforce/parse/AbstractPerforceTemplate.java‎

Lines changed: 48 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,8 +43,10 @@
4343
import com.tek42.perforce.Depot;
4444
import com.tek42.perforce.PerforceException;
4545
import com.tek42.perforce.process.Executor;
46+
import hudson.plugins.perforce.PerforceSCM;
4647
import java.io.InputStream;
4748
import org.slf4j.LoggerFactory;
49+
import org.apache.commons.lang.time.StopWatch;
4850

4951
/**
5052
* Provides default functionality for interacting with Perforce using the template design pattern.
@@ -64,6 +66,8 @@ public abstract class AbstractPerforceTemplate {
6466
"Password invalid.",
6567
"The authenticity of",
6668
};
69+
70+
private static final int P4_EXECUTOR_CHECK_PERIOD = 2000;
6771

6872
@SuppressWarnings("unused")
6973
private transient Logger logger; // Obsolete field, present just to keep demarshaller happy
@@ -310,6 +314,7 @@ protected StringBuilder getPerforceResponse(String origcmd[], ResponseFilter fil
310314
do {
311315
int mesgIndex = -1, count = 0;
312316
Executor p4 = depot.getExecFactory().newExecutor();
317+
313318
String debugCmd = "";
314319
// get entire cmd to execute
315320
String cmd[] = getExtraParams(origcmd);
@@ -328,6 +333,47 @@ protected StringBuilder getPerforceResponse(String origcmd[], ResponseFilter fil
328333

329334
try
330335
{
336+
PerforceSCM.PerforceSCMDescriptor scmDescr = PerforceSCM.getInstance();
337+
338+
if(scmDescr.hasP4ReadlineTimeout()) { // Implementation with timeout
339+
StopWatch stopWatch = new StopWatch();
340+
int timeout = scmDescr.getP4ReadLineTimeout() * 1000;
341+
stopWatch.start();
342+
343+
try {
344+
while (reader.ready() || p4.isAlive()) {
345+
if (reader.ready()) {
346+
stopWatch.reset();
347+
stopWatch.start();
348+
line = reader.readLine();
349+
if (line == null) {
350+
break;
351+
}
352+
353+
//FIXME: Remove code duplication
354+
// only check for errors if we have not found one already
355+
if (mesgIndex == -1)
356+
mesgIndex = checkAuthnErrors(line);
357+
if(filter.reject(line)) continue;
358+
lines.add(line);
359+
totalLength += line.length();
360+
count++;
361+
}
362+
else {
363+
if (stopWatch.getTime() > timeout) {
364+
throw new PerforceException("Timeout. Haven't received new line from external executor after "
365+
+scmDescr.getP4ReadLineTimeout()+" seconds. ");
366+
}
367+
368+
// Sleep for check period
369+
Thread.sleep(P4_EXECUTOR_CHECK_PERIOD);
370+
}
371+
}
372+
} catch(InterruptedException ex) {
373+
throw new PerforceException("Interrupted", ex);
374+
}
375+
376+
} else { // legacy behavior
331377
p4.getWriter().close();
332378
while((line = reader.readLine()) != null) {
333379
// only check for errors if we have not found one already
@@ -336,8 +382,9 @@ protected StringBuilder getPerforceResponse(String origcmd[], ResponseFilter fil
336382
if(filter.reject(line)) continue;
337383
lines.add(line);
338384
totalLength += line.length();
339-
count++;
385+
count++;
340386
}
387+
}
341388
}
342389
catch(IOException ioe)
343390
{

‎src/main/java/com/tek42/perforce/process/CmdLineExecutor.java‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -141,6 +141,21 @@ public Process getProcess() {
141141
return currentProcess;
142142
}
143143

144+
@Override
145+
public boolean isAlive() {
146+
if (currentProcess == null) {
147+
return false;
148+
}
149+
150+
//FIXME: Usage of exceptions isn't good approach
151+
try {
152+
int exitValue = currentProcess.exitValue();
153+
return false;
154+
} catch (IllegalThreadStateException ex) {
155+
return false;
156+
}
157+
}
158+
144159
public OutputStream getOutputStream() {
145160
throw new UnsupportedOperationException("Not supported yet.");
146161
}

‎src/main/java/com/tek42/perforce/process/Executor.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
/* * P4Java - java integration with Perforce SCM * Copyright (C) 2007-, Mike Wille, Tek42 * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public * License as published by the Free Software Foundation; either * version 2.1 of the License, or (at your option) any later version. * * This library is distributed in the hope that it will be useful, * but WITHOUT ANY WARRANTY; without even the implied warranty of * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU * Lesser General Public License for more details. * * You should have received a copy of the GNU Lesser General Public * License along with this library; if not, write to the Free Software * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA * * You can contact the author at: * * Web: http://tek42.com * Email: mike@tek42.com * Mail: 755 W Big Beaver Road * Suite 1110 * Troy, MI 48084 */package com.tek42.perforce.process;import java.io.*;import com.tek42.perforce.*;/** * A simplified interface for interacting with another process. * * @author Mike Wille */public interface Executor { /** * Execute the specified command and its arguments * * @param args * @throws PerforceException */ public void exec(String args[]) throws PerforceException; /** * Returns a BufferedWriter for writing to the stdin of this process * * @return */ public BufferedWriter getWriter(); public OutputStream getOutputStream(); /** * Returns a BufferedReader for reading from the stdout/stderr of this process * * @return */ public BufferedReader getReader(); public InputStream getInputStream(); /** * Close down all open resources */ public void close();}
1+
/* * P4Java - java integration with Perforce SCM * Copyright (C) 2007-, Mike Wille, Tek42 * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public * License as published by the Free Software Foundation; either * version 2.1 of the License, or (at your option) any later version. * * This library is distributed in the hope that it will be useful, * but WITHOUT ANY WARRANTY; without even the implied warranty of * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU * Lesser General Public License for more details. * * You should have received a copy of the GNU Lesser General Public * License along with this library; if not, write to the Free Software * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA * * You can contact the author at: * * Web: http://tek42.com * Email: mike@tek42.com * Mail: 755 W Big Beaver Road * Suite 1110 * Troy, MI 48084 */package com.tek42.perforce.process;import java.io.*;import com.tek42.perforce.*;/** * A simplified interface for interacting with another process. * * @author Mike Wille */public interface Executor { /** * Execute the specified command and its arguments * * @param args * @throws PerforceException */ public void exec(String args[]) throws PerforceException; /** * Returns a BufferedWriter for writing to the stdin of this process * * @return */ public BufferedWriter getWriter(); public OutputStream getOutputStream(); /** * Returns a BufferedReader for reading from the stdout/stderr of this process * * @return */ public BufferedReader getReader(); public InputStream getInputStream(); /** * Close down all open resources */ public void close(); /** * Check if executor stills running. * @since 1.4.0 * @throws IOException Process status check error * @throws InterruptedException Process has been interrupted */ public boolean isAlive() throws IOException, InterruptedException;}

‎src/main/java/hudson/plugins/perforce/HudsonP4DefaultExecutor.java‎

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,8 @@ public class HudsonP4DefaultExecutor implements HudsonP4Executor {
3434
private Launcher hudsonLauncher;
3535
private String[] env;
3636
private FilePath filePath;
37+
38+
private Proc currentProcess;
3739

3840
/**
3941
* Constructor that takes Hudson specific details for launching the
@@ -76,10 +78,10 @@ public void exec(String[] cmd) throws PerforceException {
7678
FastPipedOutputStream p4out = new FastPipedOutputStream(hudsonIn);
7779
output = p4out;
7880

79-
Proc process = hudsonLauncher.launch().cmds(cmd).envs(env).stdin(hudsonIn).stdout(hudsonOut).pwd(filePath).start();
81+
currentProcess = hudsonLauncher.launch().cmds(cmd).envs(env).stdin(hudsonIn).stdout(hudsonOut).pwd(filePath).start();
8082

8183
// Required to close hudsonOut stream
82-
hudsonOut.closeOnProcess(process);
84+
hudsonOut.closeOnProcess(currentProcess);
8385

8486
} catch(IOException e) {
8587
//try to close all the pipes before throwing an exception
@@ -142,4 +144,11 @@ public OutputStream getOutputStream() {
142144
return output;
143145
}
144146

147+
@Override
148+
public boolean isAlive() throws IOException, InterruptedException {
149+
return currentProcess != null ? currentProcess.isAlive() : false;
150+
}
151+
152+
153+
145154
}

‎src/main/java/hudson/plugins/perforce/HudsonP4RemoteExecutor.java‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,8 @@ public class HudsonP4RemoteExecutor implements HudsonP4Executor {
5959
private Launcher hudsonLauncher;
6060
private String[] env;
6161
private FilePath filePath;
62+
63+
private Proc currentProcess;
6264

6365
/**
6466
* Constructor that takes Hudson specific details for launching the
@@ -112,7 +114,7 @@ public void exec(String[] cmd) throws PerforceException {
112114
remotePath,
113115
listener);
114116
Future future = channel.callAsync(remoteCall);
115-
Proc proc = new RemoteProc(future);
117+
currentProcess = new RemoteProc(future);
116118

117119
} catch(IOException e) {
118120
//try to close all the pipes before throwing an exception
@@ -217,5 +219,9 @@ private void closeBuffers(){
217219
output.close();
218220
} catch(IOException ignoredException) {};
219221
}
220-
222+
223+
@Override
224+
public boolean isAlive() throws IOException, InterruptedException {
225+
return currentProcess != null ? currentProcess.isAlive() : false;
226+
}
221227
}

‎src/main/java/hudson/plugins/perforce/PerforceSCM.java‎

Lines changed: 74 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -371,6 +371,16 @@ public PerforceSCM(
371371
this.excludedFiles = Util.fixEmptyAndTrim(excludedFiles);
372372
this.excludedFilesCaseSensitivity = excludedFilesCaseSensitivity;
373373
}
374+
375+
/**
376+
* Gets instance of the PerforceSCM
377+
* @return Instance of the PerforceSCM
378+
* @since 1.4.0
379+
*/
380+
public static PerforceSCMDescriptor getInstance() {
381+
String scmName = PerforceSCM.class.getSimpleName();
382+
return (PerforceSCMDescriptor)Hudson.getInstance().getScm(scmName);
383+
}
374384

375385
/**
376386
* This only exists because we need to do initialization after we have been brought
@@ -1748,6 +1758,13 @@ private String getConcurrentClientName(FilePath workspace, String p4Client) {
17481758
@Extension
17491759
public static final class PerforceSCMDescriptor extends SCMDescriptor<PerforceSCM> {
17501760
private String p4ClientPattern;
1761+
/**
1762+
* Defines timeout for BufferedReader::readLine() operations (in seconds).
1763+
* Zero value means "infinite";
1764+
*/
1765+
private Integer p4ReadlineTimeout;
1766+
private final static int P4_INFINITE_TIMEOUT_SEC = 0;
1767+
private final static int P4_MINIMAL_TIMEOUT_SEC = 30;
17511768

17521769
public PerforceSCMDescriptor() {
17531770
super(PerforceSCM.class, PerforceRepositoryBrowser.class);
@@ -1757,7 +1774,7 @@ public PerforceSCMDescriptor() {
17571774
public String getDisplayName() {
17581775
return "Perforce";
17591776
}
1760-
1777+
17611778
@Override
17621779
public SCM newInstance(StaplerRequest req, JSONObject formData) throws FormException {
17631780
PerforceSCM newInstance = (PerforceSCM)super.newInstance(req, formData);
@@ -1810,18 +1827,58 @@ public List<PerforceToolInstallation> getP4Tools() {
18101827
return Arrays.asList(p4ToolInstallations);
18111828
}
18121829

1830+
/**
1831+
* Gets client workspace name pattern
1832+
*/
18131833
public String getP4ClientPattern() {
18141834
if (p4ClientPattern == null) {
18151835
return ".*";
18161836
} else {
18171837
return p4ClientPattern;
18181838
}
18191839
}
1840+
1841+
/**
1842+
* Gets ReadLine timeout.
1843+
* @since 1.4.0
1844+
*/
1845+
public int getP4ReadLineTimeout() {
1846+
if (p4ReadlineTimeout == null) {
1847+
return P4_INFINITE_TIMEOUT_SEC;
1848+
} else {
1849+
return p4ReadlineTimeout;
1850+
}
1851+
}
18201852

1853+
public String getP4ReadLineTimeoutStr() {
1854+
return hasP4ReadlineTimeout() ? p4ReadlineTimeout.toString() : "";
1855+
}
1856+
1857+
/**
1858+
* Checks if plugin has ReadLine timeout.
1859+
* @since 1.4.0
1860+
*/
1861+
public boolean hasP4ReadlineTimeout() {
1862+
return getP4ReadLineTimeout() != P4_INFINITE_TIMEOUT_SEC;
1863+
}
18211864

18221865
@Override
18231866
public boolean configure(StaplerRequest req, JSONObject json) throws FormException {
18241867
p4ClientPattern = Util.fixEmpty(req.getParameter("p4.clientPattern").trim());
1868+
1869+
// ReadLine timeout
1870+
String p4timeoutStr = Util.fixEmpty(req.getParameter("p4.readLineTimeout").trim());
1871+
p4ReadlineTimeout = P4_INFINITE_TIMEOUT_SEC;
1872+
if (p4timeoutStr != null)
1873+
{
1874+
try {
1875+
int val = Integer.parseInt(p4timeoutStr);
1876+
p4ReadlineTimeout = val<P4_MINIMAL_TIMEOUT_SEC ? P4_INFINITE_TIMEOUT_SEC : val;
1877+
} catch (NumberFormatException ex) {
1878+
//Do nothing - just ignore user's value
1879+
}
1880+
}
1881+
18251882
save();
18261883
return true;
18271884
}
@@ -1832,7 +1889,22 @@ public FormValidation doValidateNamePattern(StaplerRequest req) {
18321889
try {
18331890
Pattern.compile(namePattern);
18341891
} catch (PatternSyntaxException exception) {
1835-
return FormValidation.error("Pattern format error: "+exception.getMessage());
1892+
return FormValidation.error("Pattern format error:\n"+exception.getMessage());
1893+
}
1894+
}
1895+
return FormValidation.ok();
1896+
}
1897+
1898+
public FormValidation doValidateP4ReadLineTimeout(StaplerRequest req) {
1899+
String valueStr = Util.fixEmptyAndTrim(req.getParameter("value"));
1900+
if (valueStr != null) {
1901+
try {
1902+
int val = Integer.parseInt(valueStr);
1903+
if (val < P4_MINIMAL_TIMEOUT_SEC) {
1904+
return FormValidation.error("P4 ReadLine timeout should exceed "+P4_MINIMAL_TIMEOUT_SEC+" seconds. Value will be ignored");
1905+
}
1906+
} catch (NumberFormatException ex) {
1907+
return FormValidation.error("Number format error: "+ex.getMessage());
18361908
}
18371909
}
18381910
return FormValidation.ok();

‎src/main/resources/hudson/plugins/perforce/PerforceSCM/global.jelly‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,5 +33,9 @@
3333
<f:textbox name="p4.clientPattern" value="${descriptor.p4ClientPattern}"
3434
checkUrl="'${rootURL}/scm/PerforceSCM/validateNamePattern?value='+escape(this.value)"/>
3535
</f:entry>
36+
<f:entry title="P4 Read Line wait timeout, seconds" help="/plugin/perforce/help/p4ReadLineTimeout.html">
37+
<f:textbox name="p4.readLineTimeout" value="${descriptor.p4ReadLineTimeoutStr}"
38+
checkUrl="'${rootURL}/scm/PerforceSCM/validateP4ReadLineTimeout?value='+escape(this.value)"/>
39+
</f:entry>
3640
</f:section>
3741
</j:jelly>

‎src/main/webapp/help/p4ReadLineTimeout.html‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
<div>
2+
<p>Defines timeout, during which p4 plugins waits for response of p4 client. Prevents hanging of external executable</p>
3+
<p>Leave field empty in order to disable timeouts.</p>
4+
<p>See <a href="https://issues.jenkins-ci.org/browse/JENKINS-15315">JENKINS-15315</a> for more info.<p>
5+
</div>

0 commit comments

Comments
 (0)