I created a console program that tests report results from JasperReport. It retrieves the list of reports, splits it, and passes each sublist to a new thread. Each thread executes its own reports and compares the results with reference files.
My colleague says multithreading is wrong, but as usual he doesn't explain why. Any hint on what is not correct? He just spouts off something about using an inner class but was not clear, and it's hard to get more details.
This is how the code looks. I've omitted some irrelevant functions (with no side effects anyway). Every local variable is final.
public class ReportTester {
private class ThreadTest implements Runnable {
final List<Report> reports;
final Configuration config;
public ThreadTest(final List<Report> reports, Configuration config)
this.reports = reports;
this.config = config;
}
@Override
public void run() {
runTest(this.reports, this.config);
}
}
private final String format = "xml";
private final String directoryReport = "\var\reports";
private final JasperRestfulClient restClient = new JasperRestfulClient();
private final List<Report> reportsToBeTested = restClient.getReports();
volatile private errors = false; // SIDE EFFECT HERE. Public getter omitted.
private void runTest(List<Report> reports, Configuration config) {
for (Report report : reports) {
try {
String fileName = getFilePath(directoryReport, report, config);
restClient.runReport(report.getPath(), format,config, fileName);
compareWithReference(fileName, report, config);
}
catch(Exception ex){
ex.printStack();
}
}
}
public void runTestMultithreading(Configuration config, int numThread){
ExecutorService es = Executors.newCachedThreadPool();
List<List<Report>> splitted = splitReports(reportsToBeTested, numThread);
for (List<Report> reportsOfThread : splitted) {
ThreadTest thread = new ThreadTest(reportsOfThread, config);
es.execute(thread);
}
es.shutdown();
es.awaitTermination(8, TimeUnit.HOURS);
}
}
EDIT: This are the methods omitted. Actually there is a side effect, a boolean variable is assigned to true if there is at least one report that isn't identical with reference. But there is not race conditions, even without synchronization, because it can be only assigned to true, the value assigned doesn't depend on previous value of variable and it is and never read by threads.
private boolean filesAreIdentical(String filenameFirst, String filenameSecond) {
File file1 = new File(filenameFirst);
File file2 = new File(filenameSecond);
if (!file1.exists() || !file2.exists()) {
return false;
}
if (file1.length() != file2.length()) {
return false;
}
InputStream stream1 = null;
InputStream stream2 = null;
try {
stream1 = new FileInputStream(file1);
stream2 = new FileInputStream(file2);
final int BUFFSIZE = 1024;
int read1 = -1;
int read2 = -1;
byte buffer1[] = new byte[BUFFSIZE];
byte buffer2[] = new byte[BUFFSIZE];
do {
int offset1 = 0;
while (offset1 < BUFFSIZE && (read1 = stream1.read(buffer1, offset1, BUFFSIZE - offset1)) >= 0) {
offset1 += read1;
}
int offset2 = 0;
while (offset2 < BUFFSIZE && (read2 = stream2.read(buffer2, offset2, BUFFSIZE - offset2)) >= 0) {
offset2 += read2;
}
if (offset1 != offset2) {
return false;
}
if (offset1 != BUFFSIZE) {
Arrays.fill(buffer1, offset1, BUFFSIZE, (byte) 0);
Arrays.fill(buffer2, offset2, BUFFSIZE, (byte) 0);
}
if (!Arrays.equals(buffer1, buffer2)) {
return false;
}
} while (read1 >= 0 && read2 >= 0);
return read1 < 0 && read2 < 0;
} catch (IOException e) {
e.printStackTrace();
return false;
} finally {
try {
if (stream1 != null) {
stream1.close();
}
} catch (IOException e) {
e.printStackTrace();
}
try {
if (stream2 != null) {
stream2.close();
}
} catch (IOException e) {
e.printStackTrace();
}
}
}
private List<List<Report>> splitReports(List<Report> original, int number) {
List<List<Report>> sublists = new ArrayList<List<Report>>(number);
int reportsPerThread = original.size() / number;
for (int i = 0; i < number; i++) {
int start = reportsPerThread * i;
int stop = (i == number - 1)
? original.size() - 1
: reportsPerThread * (i + 1) - 1;
List<Report> sublist = createSublist(original, start, stop);
sublists.add(sublist);
}
return splits;
}
private List<Report> createSublist(final List<Report> original, int start, int stop)
{
List<Report> copy = new ArrayList<>(original.size());
if (stop > original.size() - 1) {
stop = original.size() - 1;
}
for (int ii = start; ii <= stop; ii++) {
copy.add(original.get(ii));
}
return copy;
}
private void compareWithReference(String filename, Report report, Configuration config) {
String filenameReference = directoryReport + config.subfolder() + filename;
if (filesAreIdentical(filename, filenameReference)) {
System.out.println(filename + " OK");
} else {
// SIDE EFFECT
errors = true;
System.err.println(filename + " FAILED");
}
}
This is the the client.
private final String server; //assigned by constructor
protected final CloseableHttpClient httpclient; //assigned by constructor; threadsafe
public void runReport(String url, String destinazione) throws IOException {
HttpGet httpget = null;
InputStream instream = null;
try {
File file = new File(destinazione);
file.getParentFile().mkdirs();
file.createNewFile();
httpget = new HttpGet(server + url);
HttpResponse response = httpclient.execute(httpget);
writeResponceToFile(response, file);
}
finally {
if (httpget != null) {
httpget.abort();
}
}
}
private void writeResponceToFile(HttpResponse response, File file) throws IOException {
HttpEntity entity = response.getEntity();
InputStream instream = null;
try {
if (entity != null) {
instream = entity.getContent();
BufferedInputStream bis = new BufferedInputStream(instream);
BufferedOutputStream bos = new BufferedOutputStream(new FileOutputStream(file));
int inByte;
while ((inByte = bis.read()) != -1) {
bos.write(inByte);
}
bis.close();
bos.close();
}
} finally {
if (instream != null) {
instream.close();
}
}
}
