Skip to content

Commit

Permalink
[SNAP-1537] Progress monitoring can throw IllegalArgumentException
Browse files Browse the repository at this point in the history
Introduced a synchronized progress monitor, this should prevent the exceptions.
  • Loading branch information
marpet committed Jun 23, 2022
1 parent 306e6a1 commit c0be089
Show file tree
Hide file tree
Showing 3 changed files with 186 additions and 5 deletions.
54 changes: 51 additions & 3 deletions ceres-core/src/main/java/com/bc/ceres/core/SubProgressMonitor.java
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,23 @@ public static ProgressMonitor create(ProgressMonitor monitor, int ticks) {
return new SubProgressMonitor(monitor, ticks);
}

/**
* Creates a progress monitor based on the passed in parent monitor.If the parent monitor is {@link ProgressMonitor#NULL},
* <code>monitor</code> is returned, otherwise a new <code>SubProgressMonitor</code> is created.
* In contrast to the monitor returned by {@link #create(ProgressMonitor, int)} this one can be considered to be thread-safe.
*
* @param monitor the parent progress monitor
* @param ticks the number of work ticks allocated from the
* parent monitor
* @return a progress monitor
*/
public static ProgressMonitor createSynchronized(ProgressMonitor monitor, int ticks) {
if (monitor == ProgressMonitor.NULL) {
return monitor;
}
return new SyncSubProgressMonitor(monitor, ticks);
}

/**
* Creates a new sub-progress monitor for the given monitor. The sub
* progress monitor uses the given number of work ticks from its
Expand Down Expand Up @@ -209,10 +226,41 @@ public void setSubTaskName(String subTaskName) {
}

/* (Intentionally not javadoc'd)
* Implements the method <code>ProgressMonitor.worked</code>.
*/
* Implements the method <code>ProgressMonitor.worked</code>.
*/
@Override
public void worked(int work) {
internalWorked(work);
}
}

private static class SyncSubProgressMonitor extends SubProgressMonitor {
public SyncSubProgressMonitor(ProgressMonitor monitor, int ticks) {
super(monitor, ticks);
}

@Override
public synchronized void beginTask(String taskName, int totalWork) {
super.beginTask(taskName, totalWork);
}

@Override
public synchronized void done() {
super.done();
}

@Override
public synchronized void internalWorked(double work) {
super.internalWorked(work);
}

@Override
public synchronized void setSubTaskName(String subTaskName) {
super.setSubTaskName(subTaskName);
}

@Override
public synchronized void worked(int work) {
super.worked(work);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -556,7 +556,7 @@ private static void writeBandsSequentially(ProgressMonitor pm, ArrayList<Band> b
break;
}
pm.setSubTaskName("Writing band '" + band.getName() + "'");
ProgressMonitor subPM = SubProgressMonitor.create(pm, 1);
ProgressMonitor subPM = SubProgressMonitor.createSynchronized(pm, 1);
writeRasterDataFully(subPM, band, null, null, null);
}
}
Expand Down Expand Up @@ -597,7 +597,11 @@ private static void writeRasterDataFully(ProgressMonitor pm, Band band, Executor
ioExceptionCollector.add(e);
pm.setCanceled(true);
} finally {
finisher.worked();
try {
finisher.worked();
} catch (IllegalArgumentException e) {
e.printStackTrace();
}
}
});
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,129 @@
/*
* Copyright (c) 2022. Brockmann Consult GmbH (info@brockmann-consult.de)
*
* This program is free software; you can redistribute it and/or modify it
* under the terms of the GNU General Public License as published by the Free
* Software Foundation; either version 3 of the License, or (at your option)
* any later version.
* This program 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 General Public License for
* more details.
*
* You should have received a copy of the GNU General Public License along
* with this program; if not, see http://www.gnu.org/licenses/
*
*
*/

package org.esa.snap.dataio.znap;

import org.esa.snap.core.dataio.ProductIO;
import org.esa.snap.core.datamodel.Band;
import org.esa.snap.core.datamodel.Product;
import org.esa.snap.core.datamodel.ProductData;
import org.esa.snap.core.util.io.TreeDeleter;
import org.esa.snap.runtime.Config;
import org.junit.After;
import org.junit.AfterClass;
import org.junit.Before;
import org.junit.BeforeClass;
import org.junit.Ignore;
import org.junit.Test;

import javax.media.jai.operator.ConstantDescriptor;
import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.prefs.Preferences;

import static org.esa.snap.dataio.znap.preferences.ZnapPreferencesConstants.PROPERTY_NAME_USE_ZIP_ARCHIVE;


/**
* @author Marco Peters
*/
@Ignore("Should only be used locally to test issues with threading. Enable num executes in Idea to execute the test multiple times.")
public class ProgressMonitoringThreadSafeteyTest {
private static Path baseTestPath;
private Product dummy;
private static String useZipArchiveBackup;

@BeforeClass
public static void beforeClass() throws Exception {
final String tmpDir = System.getProperty("java.io.tmpdir");
// deleting temp directory in @After or @AfterClass method didn't work reliable. Probably sometimes some
// files were still in use at the time delete was called. So we try, delete on exit with a shutdown hook.
// We have one common test dir. Each test creates its own folder
baseTestPath = Paths.get(tmpDir, ZnapProductWriterTest_writeToArchiveOrFolder.class.getCanonicalName());
deleteRemainingsOfPreviousRun();
Files.createDirectories(baseTestPath);

Runtime.getRuntime().addShutdownHook(new Thread(() -> {
try {
TreeDeleter.deleteDir(baseTestPath);
} catch (IOException ignore) {
}
}));
}

private static void deleteRemainingsOfPreviousRun() throws IOException {
if (Files.isDirectory(baseTestPath)) {
TreeDeleter.deleteDir(baseTestPath);
}
}

@Before
public void setUp() throws Exception {
dummy = createDummyProduct();
final Preferences prefs = Config.instance("snap").load().preferences();
useZipArchiveBackup = prefs.get(PROPERTY_NAME_USE_ZIP_ARCHIVE, null);
prefs.put(PROPERTY_NAME_USE_ZIP_ARCHIVE, "false");

}

@After
public void tearDown() {
try {
TreeDeleter.deleteDir(baseTestPath);
} catch (IOException ignore) {
}
}

@AfterClass
public static void afterClass() throws Exception {
final Preferences prefs = Config.instance("snap").load().preferences();
if (useZipArchiveBackup == null) {
prefs.remove(PROPERTY_NAME_USE_ZIP_ARCHIVE);
} else {
prefs.put(PROPERTY_NAME_USE_ZIP_ARCHIVE, useZipArchiveBackup);
}
}

@Test
public void testMethod() throws IOException {
final Path testDir = baseTestPath.resolve("writeToFolder");
ProductIO.writeProduct(dummy, testDir.resolve("testName").toFile(), "ZNAP", false);
}

private Product createDummyProduct() {
final Product targetProduct = new Product("name", "type");
for (int i = 0; i < 25; i++) {
Band band = new Band("band_0" + i, ProductData.TYPE_INT32, 5000, 2000);
band.setSourceImage(ConstantDescriptor.create(5000f, 2000f, new Integer[]{i}, null));
targetProduct.addBand(band);
}
for (int i = 0; i < 50; i++) {
Band band = new Band("band_1" + i, ProductData.TYPE_INT32, 1000, 200);
band.setSourceImage(ConstantDescriptor.create(1000f, 200f, new Integer[]{i}, null));
targetProduct.addBand(band);
}
for (int i = 0; i < 25; i++) {
Band band = new Band("band_2" + i, ProductData.TYPE_INT32, 5000, 2000);
band.setSourceImage(ConstantDescriptor.create(5000f, 2000f, new Integer[]{i}, null));
targetProduct.addBand(band);
}
return targetProduct;
}
}

0 comments on commit c0be089

Please sign in to comment.