This is an automated email from the ASF dual-hosted git repository. desruisseaux pushed a commit to branch geoapi-4.0 in repository https://gitbox.apache.org/repos/asf/sis.git
The following commit(s) were added to refs/heads/geoapi-4.0 by this push: new c7c5901b81 Allow data stores to be closed asynchronously. https://issues.apache.org/jira/browse/SIS-573 c7c5901b81 is described below commit c7c5901b81e00d5843ebb2688c017d4f99424bd0 Author: Martin Desruisseaux <martin.desruisse...@geomatys.com> AuthorDate: Sat Feb 11 15:51:05 2023 +0100 Allow data stores to be closed asynchronously. https://issues.apache.org/jira/browse/SIS-573 --- .../apache/sis/internal/gui/BackgroundThreads.java | 13 +-- .../apache/sis/internal/gui/DataStoreOpener.java | 59 ++++------- .../apache/sis/internal/system/CommonExecutor.java | 2 +- .../apache/sis/storage/landsat/LandsatStore.java | 56 ++++++---- .../apache/sis/storage/landsat/package-info.java | 2 +- .../apache/sis/storage/geotiff/GeoTiffStore.java | 26 +++-- .../org/apache/sis/storage/geotiff/Reader.java | 1 + .../org/apache/sis/internal/netcdf/Decoder.java | 11 +- .../sis/internal/netcdf/impl/ChannelDecoder.java | 7 +- .../sis/internal/netcdf/impl/package-info.java | 2 +- .../sis/internal/netcdf/ucar/DecoderWrapper.java | 20 ++-- .../sis/internal/netcdf/ucar/package-info.java | 2 +- .../org/apache/sis/storage/netcdf/NetcdfStore.java | 22 ++-- .../org/apache/sis/internal/netcdf/TestCase.java | 6 +- .../sis/storage/netcdf/MetadataReaderTest.java | 17 ++-- .../storage/netcdf/NetcdfStoreProviderTest.java | 15 +-- .../sis/internal/storage/GridResourceWrapper.java | 26 ++--- .../sis/internal/storage/csv/package-info.java | 2 +- .../sis/internal/storage/esri/AsciiGridStore.java | 29 ++++-- .../sis/internal/storage/esri/RasterStore.java | 2 +- .../sis/internal/storage/esri/RawRasterStore.java | 32 +++--- .../sis/internal/storage/esri/WritableStore.java | 3 + .../internal/storage/folder/ConcurrentCloser.java | 113 +++++++++++++++++++++ .../apache/sis/internal/storage/folder/Store.java | 38 ++++--- .../sis/internal/storage/image/WorldFileStore.java | 50 +++++---- .../sis/internal/storage/image/WritableStore.java | 3 + .../internal/storage/io/FileCacheByteChannel.java | 23 +++-- .../org/apache/sis/internal/storage/wkt/Store.java | 25 +++-- .../org/apache/sis/internal/storage/xml/Store.java | 24 +++-- .../java/org/apache/sis/storage/DataStore.java | 5 + .../apache/sis/storage/event/StoreListeners.java | 9 +- .../org/apache/sis/internal/storage/gpx/Store.java | 3 + .../internal/storage/xml/stream/StaxDataStore.java | 27 +++-- 33 files changed, 431 insertions(+), 244 deletions(-) diff --git a/application/sis-javafx/src/main/java/org/apache/sis/internal/gui/BackgroundThreads.java b/application/sis-javafx/src/main/java/org/apache/sis/internal/gui/BackgroundThreads.java index 0b9e25e1b9..10f333b9e0 100644 --- a/application/sis-javafx/src/main/java/org/apache/sis/internal/gui/BackgroundThreads.java +++ b/application/sis-javafx/src/main/java/org/apache/sis/internal/gui/BackgroundThreads.java @@ -29,6 +29,7 @@ import java.util.concurrent.atomic.AtomicInteger; import javafx.application.Platform; import org.apache.sis.gui.DataViewer; import org.apache.sis.internal.system.Threads; +import org.apache.sis.storage.DataStoreException; import org.apache.sis.util.logging.Logging; import org.apache.sis.util.Exceptions; @@ -171,20 +172,16 @@ public final class BackgroundThreads extends AtomicInteger implements ThreadFact * This method returns soon but the background threads may continue for some time if they did not finished * their task yet. * - * @throws Exception if an error occurred while closing at least one data store. + * @throws DataStoreException if an error occurred while closing at least one data store. */ - public static void stop() throws Exception { - EXECUTOR.shutdown(); + public static void stop() throws DataStoreException { + EXECUTOR.shutdown(); // Prevent scheduling of more tasks. + DataStoreOpener.closeAll(); // Throws AsynchronousCloseException in threads that are reading. try { EXECUTOR.awaitTermination(30, TimeUnit.SECONDS); } catch (InterruptedException e) { - /* - * Someone does not want to wait for termination. - * Closes the data stores now even if some of them may still be in use. - */ interrupted("stop", e); } - DataStoreOpener.closeAll(); } /** diff --git a/application/sis-javafx/src/main/java/org/apache/sis/internal/gui/DataStoreOpener.java b/application/sis-javafx/src/main/java/org/apache/sis/internal/gui/DataStoreOpener.java index 84e41c114e..73424a65f4 100644 --- a/application/sis-javafx/src/main/java/org/apache/sis/internal/gui/DataStoreOpener.java +++ b/application/sis-javafx/src/main/java/org/apache/sis/internal/gui/DataStoreOpener.java @@ -22,11 +22,12 @@ import java.io.File; import java.nio.file.Path; import java.io.IOException; import java.net.URISyntaxException; +import java.nio.file.FileSystemNotFoundException; +import java.util.List; import java.util.Locale; import java.util.Collection; -import java.util.function.Consumer; +import java.util.concurrent.Callable; import java.util.function.UnaryOperator; -import java.util.stream.Stream; import javafx.concurrent.Task; import javafx.event.EventHandler; import javafx.application.Platform; @@ -47,6 +48,7 @@ import org.apache.sis.internal.util.Strings; import org.apache.sis.internal.storage.io.IOUtilities; import org.apache.sis.internal.storage.io.ChannelFactory; import org.apache.sis.internal.storage.io.InternalOptionKey; +import org.apache.sis.internal.storage.folder.ConcurrentCloser; import org.apache.sis.storage.DataStore; import org.apache.sis.gui.DataViewer; @@ -72,7 +74,7 @@ import org.apache.sis.gui.DataViewer; * @todo Set title. Add progress listener and cancellation capability. * * @author Martin Desruisseaux (Geomatys) - * @version 1.3 + * @version 1.4 * * @see BackgroundThreads#execute(Runnable) * @@ -140,9 +142,9 @@ public final class DataStoreOpener extends Task<DataStore> { source = ((Path) source).toRealPath(); // May throw IOException. } } - } catch (URISyntaxException | IOException | IllegalArgumentException e) { + } catch (URISyntaxException | FileSystemNotFoundException | IllegalArgumentException e) { // Ignore — keep `source` as is (File, URI, URI or non-absolute Path). - } catch (DataStoreException | RuntimeException e) { + } catch (DataStoreException | IOException | RuntimeException e) { source = null; } key = source; @@ -352,43 +354,24 @@ public final class DataStoreOpener extends Task<DataStore> { * terminated in case some of them were using a data store. The data stores will be closed * in parallel. * - * @throws Exception if an error occurred while closing at least one data store. + * @throws DataStoreException if an error occurred while closing at least one data store. */ - static void closeAll() throws Exception { - final Closer closer = new Closer(); - do { - // Use `toArray()` because we need a snapshot. - Stream.of(CACHE.keySet().toArray()).parallel().forEach(closer); - } while (!CACHE.isEmpty()); - closer.rethrow(); + static void closeAll() throws DataStoreException { + do CLOSER.closeAll(List.copyOf(CACHE.keySet())); + while (!CACHE.isEmpty()); } /** - * The handler in charge of closing the data store and record the failures if some errors happen. - * The same handler instance may be used concurrently while closing many data stores in parallel. + * Helper for closing concurrently the stores. */ - private static final class Closer implements Consumer<Object> { - /** The error that occurred while closing a data store. */ - private Exception error; - - /** Closes the given data store. */ - @Override public void accept(final Object source) { - final DataStore toClose = CACHE.remove(source); - if (source != null) try { - toClose.close(); - } catch (Exception e) { - synchronized (this) { - if (error == null) error = e; - else error.addSuppressed(e); - } - } + public static final ConcurrentCloser<Object> CLOSER = new ConcurrentCloser<>() { + @Override protected Callable<?> closer(final Object key) { + final DataStore store = CACHE.remove(key); + if (store != null) return () -> { + store.close(); + return null; + }; + return null; } - - /** If an error occurred, re-throws that error. */ - synchronized void rethrow() throws Exception { - if (error != null) { - throw error; - } - } - } + }; } diff --git a/core/sis-utility/src/main/java/org/apache/sis/internal/system/CommonExecutor.java b/core/sis-utility/src/main/java/org/apache/sis/internal/system/CommonExecutor.java index 17b5e7b6ac..8b8e47e837 100644 --- a/core/sis-utility/src/main/java/org/apache/sis/internal/system/CommonExecutor.java +++ b/core/sis-utility/src/main/java/org/apache/sis/internal/system/CommonExecutor.java @@ -27,7 +27,7 @@ import java.util.concurrent.atomic.AtomicInteger; /** * The executor shared by most of Apache SIS library for relatively "heavy" operations. - * The operations should relatively long tasks, otherwise work-stealing algorithms may + * The operations should be relatively long tasks, otherwise work-stealing algorithms may * provide better performances. For example, it may be used when each computational unit * is an image tile, in which case the thread scheduling overhead is small compared to * the size of the computational task. diff --git a/storage/sis-earth-observation/src/main/java/org/apache/sis/storage/landsat/LandsatStore.java b/storage/sis-earth-observation/src/main/java/org/apache/sis/storage/landsat/LandsatStore.java index e2ef1b4ebb..60c3117d6f 100644 --- a/storage/sis-earth-observation/src/main/java/org/apache/sis/storage/landsat/LandsatStore.java +++ b/storage/sis-earth-observation/src/main/java/org/apache/sis/storage/landsat/LandsatStore.java @@ -19,6 +19,7 @@ package org.apache.sis.storage.landsat; import java.util.Map; import java.util.List; import java.util.Optional; +import java.util.concurrent.Callable; import java.io.Reader; import java.io.BufferedReader; import java.io.LineNumberReader; @@ -45,6 +46,7 @@ import org.apache.sis.storage.event.StoreEvent; import org.apache.sis.storage.event.StoreListener; import org.apache.sis.storage.event.WarningEvent; import org.apache.sis.internal.storage.URIDataStore; +import org.apache.sis.internal.storage.folder.ConcurrentCloser; import org.apache.sis.internal.system.DefaultFactories; import org.apache.sis.internal.util.UnmodifiableArrayList; import org.apache.sis.setup.OptionKey; @@ -78,7 +80,7 @@ import org.apache.sis.setup.OptionKey; * * @author Thi Phuong Hao Nguyen (VNSC) * @author Martin Desruisseaux (Geomatys) - * @version 1.3 + * @version 1.4 * @since 1.1 */ public class LandsatStore extends DataStore implements Aggregate { @@ -111,7 +113,8 @@ public class LandsatStore extends DataStore implements Aggregate { * The array of aggregates for each Landsat band group, or {@code null} if not yet created. * This array is created together with {@linkplain #metadata} and is unmodifiable. */ - private BandGroup[] components; + @SuppressWarnings("VolatileArrayField") // Array elements are not modified after creation. + private volatile BandGroup[] components; /** * Creates a new Landsat store from the given file, URL, stream or character reader. @@ -203,7 +206,7 @@ public class LandsatStore extends DataStore implements Aggregate { * Parses the main Landsat text file. * Also creates the array of components, but without loading GeoTIFF data yet. */ - private void loadMetadata() throws DataStoreException { + private BandGroup[] loadMetadata() throws DataStoreException { if (source == null) { throw new DataStoreClosedException(getLocale(), LandsatStoreProvider.NAME, StandardOpenOption.READ); } @@ -234,10 +237,12 @@ public class LandsatStore extends DataStore implements Aggregate { } catch (FactoryException e) { throw new DataStoreReferencingException(e); } - components = BandGroup.group(listeners, resources, count); - for (final BandGroup c : components) { + final BandGroup[] bands = BandGroup.group(listeners, resources, count); + for (final BandGroup c : bands) { c.identifier = factory.createLocalName(scope, c.group.name()); } + components = bands; + return bands; } /** @@ -266,10 +271,11 @@ public class LandsatStore extends DataStore implements Aggregate { */ @Override public synchronized List<Aggregate> components() throws DataStoreException { - if (components == null) { - loadMetadata(); + BandGroup[] bands = components; + if (bands == null) { + bands = loadMetadata(); } - return UnmodifiableArrayList.wrap(components); + return UnmodifiableArrayList.wrap(bands); } /** @@ -287,26 +293,32 @@ public class LandsatStore extends DataStore implements Aggregate { /** * Closes this Landsat store and releases any underlying resources. + * This method can be invoked asynchronously for interrupting a long reading process. * * @throws DataStoreException if an error occurred while closing the Landsat file. */ @Override - public synchronized void close() throws DataStoreException { + public void close() throws DataStoreException { listeners.close(); // Should never fail. - metadata = null; - DataStoreException error = null; - for (final Band band : BandGroup.bands(components)) { - try { - band.closeDataStore(); - } catch (DataStoreException e) { - if (error == null) { - error = e; - } else { - error.addSuppressed(e); - } + try { + CLOSER.closeAll(BandGroup.bands(components)); + } finally { + synchronized (this) { + metadata = null; + components = null; } } - components = null; - if (error != null) throw error; } + + /** + * Helper for closing concurrently the images for each band. + */ + private static final ConcurrentCloser<Band> CLOSER = new ConcurrentCloser<>() { + @Override protected Callable<?> closer(final Band r) { + return () -> { + r.closeDataStore(); + return null; + }; + } + }; } diff --git a/storage/sis-earth-observation/src/main/java/org/apache/sis/storage/landsat/package-info.java b/storage/sis-earth-observation/src/main/java/org/apache/sis/storage/landsat/package-info.java index 5149314602..93e964fb18 100644 --- a/storage/sis-earth-observation/src/main/java/org/apache/sis/storage/landsat/package-info.java +++ b/storage/sis-earth-observation/src/main/java/org/apache/sis/storage/landsat/package-info.java @@ -27,7 +27,7 @@ * @author Thi Phuong Hao Nguyen (VNSC) * @author Minh Chinh Vu (VNSC) * @author Martin Desruisseaux (Geomatys) - * @version 1.3 + * @version 1.4 * @since 1.1 */ package org.apache.sis.storage.landsat; diff --git a/storage/sis-geotiff/src/main/java/org/apache/sis/storage/geotiff/GeoTiffStore.java b/storage/sis-geotiff/src/main/java/org/apache/sis/storage/geotiff/GeoTiffStore.java index cc01325ed6..218f2bb4f1 100644 --- a/storage/sis-geotiff/src/main/java/org/apache/sis/storage/geotiff/GeoTiffStore.java +++ b/storage/sis-geotiff/src/main/java/org/apache/sis/storage/geotiff/GeoTiffStore.java @@ -68,7 +68,7 @@ import org.apache.sis.util.ArgumentChecks; * @author Martin Desruisseaux (Geomatys) * @author Thi Phuong Hao Nguyen (VNSC) * @author Alexis Manin (Geomatys) - * @version 1.3 + * @version 1.4 * @since 0.8 */ public class GeoTiffStore extends DataStore implements Aggregate { @@ -83,7 +83,7 @@ public class GeoTiffStore extends DataStore implements Aggregate { * * @see #reader() */ - private Reader reader; + private volatile Reader reader; /** * The {@link GeoTiffStoreProvider#LOCATION} parameter value, or {@code null} if none. @@ -220,6 +220,7 @@ public class GeoTiffStore extends DataStore implements Aggregate { * This method must be invoked inside a block synchronized on {@code this}. */ final NameSpace namespace() { + final Reader reader = this.reader; if (!isNamespaceSet && reader != null) { final NameFactory f = reader.nameFactory; GenericName name = null; @@ -537,19 +538,26 @@ public class GeoTiffStore extends DataStore implements Aggregate { /** * Closes this GeoTIFF store and releases any underlying resources. + * This method can be invoked asynchronously for interrupting a long reading process. * * @throws DataStoreException if an error occurred while closing the GeoTIFF file. */ @Override - public synchronized void close() throws DataStoreException { - listeners.close(); // Should never fail. - final Reader r = reader; - reader = null; - components = null; - if (r != null) try { - r.close(); + public void close() throws DataStoreException { + try { + listeners.close(); // Should never fail. + final Reader r = reader; + if (r != null) r.close(); } catch (IOException e) { throw new DataStoreException(e); + } finally { + synchronized (this) { + components = null; + namespace = null; + metadata = null; + nativeMetadata = null; + reader = null; + } } } diff --git a/storage/sis-geotiff/src/main/java/org/apache/sis/storage/geotiff/Reader.java b/storage/sis-geotiff/src/main/java/org/apache/sis/storage/geotiff/Reader.java index 6cee41a8b5..01c4257a13 100644 --- a/storage/sis-geotiff/src/main/java/org/apache/sis/storage/geotiff/Reader.java +++ b/storage/sis-geotiff/src/main/java/org/apache/sis/storage/geotiff/Reader.java @@ -453,6 +453,7 @@ final class Reader extends GeoTIFF { /** * Closes this reader. + * This method can be invoked asynchronously for interrupting a long reading process. * * @throws IOException if an error occurred while closing this reader. */ diff --git a/storage/sis-netcdf/src/main/java/org/apache/sis/internal/netcdf/Decoder.java b/storage/sis-netcdf/src/main/java/org/apache/sis/internal/netcdf/Decoder.java index 676c79abea..a07c219491 100644 --- a/storage/sis-netcdf/src/main/java/org/apache/sis/internal/netcdf/Decoder.java +++ b/storage/sis-netcdf/src/main/java/org/apache/sis/internal/netcdf/Decoder.java @@ -28,7 +28,6 @@ import java.util.concurrent.TimeUnit; import java.util.logging.LogRecord; import java.util.logging.Logger; import java.util.logging.Level; -import java.io.Closeable; import java.io.IOException; import java.nio.file.Path; import org.opengis.util.NameSpace; @@ -61,7 +60,7 @@ import ucar.nc2.constants.CF; * @version 1.4 * @since 0.3 */ -public abstract class Decoder extends ReferencingFactoryContainer implements Closeable { +public abstract class Decoder extends ReferencingFactoryContainer { /** * The logger to use for messages other than warnings specific to the file being read. * This is rarely used directly because {@code listeners.getLogger()} should be preferred. @@ -525,4 +524,12 @@ public abstract class Decoder extends ReferencingFactoryContainer implements Clo final Resources resources() { return Resources.forLocale(listeners.getLocale()); } + + /** + * Closes this decoder and releases resources. + * + * @param lock the lock to use in {@code synchronized(lock)} statements. + * @throws IOException if an error occurred while closing the decoder. + */ + public abstract void close(DataStore lock) throws IOException; } diff --git a/storage/sis-netcdf/src/main/java/org/apache/sis/internal/netcdf/impl/ChannelDecoder.java b/storage/sis-netcdf/src/main/java/org/apache/sis/internal/netcdf/impl/ChannelDecoder.java index 0642762de2..1400357018 100644 --- a/storage/sis-netcdf/src/main/java/org/apache/sis/internal/netcdf/impl/ChannelDecoder.java +++ b/storage/sis-netcdf/src/main/java/org/apache/sis/internal/netcdf/impl/ChannelDecoder.java @@ -53,6 +53,7 @@ import org.apache.sis.internal.storage.io.ChannelDataInput; import org.apache.sis.internal.util.Constants; import org.apache.sis.internal.util.CollectionsExt; import org.apache.sis.internal.util.StandardDateFormat; +import org.apache.sis.storage.DataStore; import org.apache.sis.storage.DataStoreException; import org.apache.sis.storage.DataStoreContentException; import org.apache.sis.storage.event.StoreListeners; @@ -74,7 +75,7 @@ import org.apache.sis.math.Vector; * * @author Johann Sorel (Geomatys) * @author Martin Desruisseaux (Geomatys) - * @version 1.3 + * @version 1.4 * * @see <a href="http://portal.opengeospatial.org/files/?artifact_id=43734">NetCDF Classic and 64-bit Offset Format (1.0)</a> * @@ -1046,11 +1047,13 @@ nextVar: for (final VariableInfo variable : variables) { /** * Closes the channel. + * This method can be invoked asynchronously for interrupting a long reading process. * + * @param lock ignored because this method can be run asynchronously. * @throws IOException if an error occurred while closing the channel. */ @Override - public void close() throws IOException { + public void close(final DataStore lock) throws IOException { input.channel.close(); } diff --git a/storage/sis-netcdf/src/main/java/org/apache/sis/internal/netcdf/impl/package-info.java b/storage/sis-netcdf/src/main/java/org/apache/sis/internal/netcdf/impl/package-info.java index 30825f10a6..2ab63c26b9 100644 --- a/storage/sis-netcdf/src/main/java/org/apache/sis/internal/netcdf/impl/package-info.java +++ b/storage/sis-netcdf/src/main/java/org/apache/sis/internal/netcdf/impl/package-info.java @@ -30,7 +30,7 @@ * * @author Johann Sorel (Geomatys) * @author Martin Desruisseaux (Geomatys) - * @version 1.3 + * @version 1.4 * @since 0.3 */ package org.apache.sis.internal.netcdf.impl; diff --git a/storage/sis-netcdf/src/main/java/org/apache/sis/internal/netcdf/ucar/DecoderWrapper.java b/storage/sis-netcdf/src/main/java/org/apache/sis/internal/netcdf/ucar/DecoderWrapper.java index e29dc154d8..14bacb7e98 100644 --- a/storage/sis-netcdf/src/main/java/org/apache/sis/internal/netcdf/ucar/DecoderWrapper.java +++ b/storage/sis-netcdf/src/main/java/org/apache/sis/internal/netcdf/ucar/DecoderWrapper.java @@ -61,16 +61,13 @@ import org.apache.sis.storage.event.StoreListeners; * Provides netCDF decoding services based on the netCDF library. * * @author Martin Desruisseaux (Geomatys) - * @version 1.3 + * @version 1.4 * @since 0.3 */ public final class DecoderWrapper extends Decoder implements CancelTask { /** * The netCDF file to read. * This file is set at construction time. - * - * <p>This {@code DecoderWrapper} class does <strong>not</strong> close this file. - * Closing this file after usage is the user responsibility.</p> */ private final NetcdfFile file; @@ -93,7 +90,7 @@ public final class DecoderWrapper extends Decoder implements CancelTask { /** * The discrete sampling features or grids found by UCAR library, or {@code null} if none. - * This reference is kept for making possible to close it in {@link #close()}. + * This reference is kept for making possible to close it in {@link #close(DataStore)}. * * @see #getDiscreteSampling(Object) */ @@ -668,15 +665,18 @@ public final class DecoderWrapper extends Decoder implements CancelTask { /** * Closes the netCDF file. * + * @param lock the lock to use in {@code synchronized(lock)} statements. * @throws IOException if an error occurred while closing the file. */ @Override - public void close() throws IOException { - if (features != null) { - features.close(); - features = null; + public void close(final DataStore lock) throws IOException { + synchronized (lock) { + if (features != null) { + features.close(); + features = null; + } + file.close(); } - file.close(); } /** diff --git a/storage/sis-netcdf/src/main/java/org/apache/sis/internal/netcdf/ucar/package-info.java b/storage/sis-netcdf/src/main/java/org/apache/sis/internal/netcdf/ucar/package-info.java index 533a973f6f..6f7ee16ea1 100644 --- a/storage/sis-netcdf/src/main/java/org/apache/sis/internal/netcdf/ucar/package-info.java +++ b/storage/sis-netcdf/src/main/java/org/apache/sis/internal/netcdf/ucar/package-info.java @@ -20,7 +20,7 @@ * as wrappers around the UCAR netCDF library. * * @author Martin Desruisseaux (IRD, Geomatys) - * @version 1.3 + * @version 1.4 * @since 0.3 */ package org.apache.sis.internal.netcdf.ucar; diff --git a/storage/sis-netcdf/src/main/java/org/apache/sis/storage/netcdf/NetcdfStore.java b/storage/sis-netcdf/src/main/java/org/apache/sis/storage/netcdf/NetcdfStore.java index fcfacfaf30..7b0a2c0950 100644 --- a/storage/sis-netcdf/src/main/java/org/apache/sis/storage/netcdf/NetcdfStore.java +++ b/storage/sis-netcdf/src/main/java/org/apache/sis/storage/netcdf/NetcdfStore.java @@ -261,20 +261,26 @@ public class NetcdfStore extends DataStore implements Aggregate { /** * Closes this netCDF store and releases any underlying resources. + * This method can be invoked asynchronously for interrupting a long reading process. * * @throws DataStoreException if an error occurred while closing the netCDF file. */ @Override - public synchronized void close() throws DataStoreException { - listeners.close(); // Should never fail. - final Decoder reader = decoder; - decoder = null; - metadata = null; - components = null; - if (reader != null) try { - reader.close(); + public void close() throws DataStoreException { + try { + listeners.close(); // Should never fail. + final Decoder reader = decoder; + if (reader != null) { + reader.close(this); + } } catch (IOException e) { throw new DataStoreException(e); + } finally { + synchronized (this) { + components = null; + metadata = null; + decoder = null; + } } } diff --git a/storage/sis-netcdf/src/test/java/org/apache/sis/internal/netcdf/TestCase.java b/storage/sis-netcdf/src/test/java/org/apache/sis/internal/netcdf/TestCase.java index 65308fb37b..ebe37c7989 100644 --- a/storage/sis-netcdf/src/test/java/org/apache/sis/internal/netcdf/TestCase.java +++ b/storage/sis-netcdf/src/test/java/org/apache/sis/internal/netcdf/TestCase.java @@ -27,6 +27,7 @@ import org.apache.sis.storage.DataStoreException; import org.apache.sis.internal.netcdf.ucar.DecoderWrapper; import org.apache.sis.setup.GeometryLibrary; import org.apache.sis.storage.event.StoreListeners; +import org.apache.sis.storage.DataStoreMock; import org.opengis.test.dataset.TestData; import ucar.nc2.dataset.NetcdfDataset; import ucar.nc2.NetcdfFile; @@ -42,7 +43,7 @@ import static org.junit.Assert.*; * <p>This class is <strong>not</strong> thread safe - do not run subclasses in parallel.</p> * * @author Martin Desruisseaux (Geomatys) - * @version 1.2 + * @version 1.4 * @since 0.3 */ public abstract class TestCase extends org.apache.sis.test.TestCase { @@ -175,13 +176,14 @@ public abstract class TestCase extends org.apache.sis.test.TestCase { */ @AfterClass public static void closeAllDecoders() throws IOException { + final var ds = new DataStoreMock("lock"); Throwable failure = null; synchronized (DECODERS) { // Paranoiac safety. final Iterator<Decoder> it = DECODERS.values().iterator(); while (it.hasNext()) { final Decoder decoder = it.next(); try { - decoder.close(); + decoder.close(ds); } catch (Throwable e) { if (failure == null) { failure = e; diff --git a/storage/sis-netcdf/src/test/java/org/apache/sis/storage/netcdf/MetadataReaderTest.java b/storage/sis-netcdf/src/test/java/org/apache/sis/storage/netcdf/MetadataReaderTest.java index c6c740199d..df2b33fbbb 100644 --- a/storage/sis-netcdf/src/test/java/org/apache/sis/storage/netcdf/MetadataReaderTest.java +++ b/storage/sis-netcdf/src/test/java/org/apache/sis/storage/netcdf/MetadataReaderTest.java @@ -33,6 +33,7 @@ import org.apache.sis.internal.netcdf.TestCase; import org.apache.sis.internal.netcdf.Decoder; import org.apache.sis.internal.netcdf.impl.ChannelDecoderTest; import org.apache.sis.storage.DataStoreException; +import org.apache.sis.storage.DataStoreMock; import org.apache.sis.test.DependsOn; import org.junit.Test; @@ -45,7 +46,7 @@ import static org.apache.sis.test.TestUtilities.date; * for reading netCDF attributes. * * @author Martin Desruisseaux (Geomatys) - * @version 1.2 + * @version 1.4 * @since 0.3 */ @DependsOn({ @@ -71,10 +72,9 @@ public final class MetadataReaderTest extends TestCase { */ @Test public void testEmbedded() throws IOException, DataStoreException { - final Metadata metadata; - try (Decoder input = ChannelDecoderTest.createChannelDecoder(TestData.NETCDF_2D_GEOGRAPHIC)) { - metadata = new MetadataReader(input).read(); - } + final Decoder input = ChannelDecoderTest.createChannelDecoder(TestData.NETCDF_2D_GEOGRAPHIC); + final Metadata metadata = new MetadataReader(input).read(); + input.close(new DataStoreMock("lock")); compareToExpected(metadata).assertMetadataEquals(); } @@ -87,10 +87,9 @@ public final class MetadataReaderTest extends TestCase { */ @Test public void testUCAR() throws IOException, DataStoreException { - final Metadata metadata; - try (Decoder input = createDecoder(TestData.NETCDF_2D_GEOGRAPHIC)) { - metadata = new MetadataReader(input).read(); - } + final Decoder input = createDecoder(TestData.NETCDF_2D_GEOGRAPHIC); + final Metadata metadata = new MetadataReader(input).read(); + input.close(new DataStoreMock("lock")); final ContentVerifier verifier = compareToExpected(metadata); verifier.addExpectedValue("identificationInfo[0].resourceFormat[0].formatSpecificationCitation.alternateTitle[1]", "NetCDF-3/CDM"); verifier.assertMetadataEquals(); diff --git a/storage/sis-netcdf/src/test/java/org/apache/sis/storage/netcdf/NetcdfStoreProviderTest.java b/storage/sis-netcdf/src/test/java/org/apache/sis/storage/netcdf/NetcdfStoreProviderTest.java index e7f8c75eb5..c31b9ae8c8 100644 --- a/storage/sis-netcdf/src/test/java/org/apache/sis/storage/netcdf/NetcdfStoreProviderTest.java +++ b/storage/sis-netcdf/src/test/java/org/apache/sis/storage/netcdf/NetcdfStoreProviderTest.java @@ -26,6 +26,7 @@ import org.apache.sis.internal.netcdf.impl.ChannelDecoderTest; import org.apache.sis.storage.ProbeResult; import org.apache.sis.storage.StorageConnector; import org.apache.sis.storage.DataStoreException; +import org.apache.sis.storage.DataStoreMock; import org.apache.sis.util.Version; import org.apache.sis.test.DependsOn; import org.opengis.test.dataset.TestData; @@ -38,7 +39,7 @@ import static org.opengis.test.Assert.*; * Tests {@link NetcdfStoreProvider}. * * @author Martin Desruisseaux (Geomatys) - * @version 1.0 + * @version 1.4 * @since 0.3 */ @DependsOn({ @@ -90,9 +91,9 @@ public final class NetcdfStoreProviderTest extends TestCase { @Test public void testDecoderFromStream() throws IOException, DataStoreException { final StorageConnector c = new StorageConnector(TestData.NETCDF_2D_GEOGRAPHIC.open()); - try (Decoder decoder = NetcdfStoreProvider.decoder(createListeners(), c)) { - assertInstanceOf("decoder", ChannelDecoder.class, decoder); - } + final Decoder decoder = NetcdfStoreProvider.decoder(createListeners(), c); + assertInstanceOf("decoder", ChannelDecoder.class, decoder); + decoder.close(new DataStoreMock("lock")); } /** @@ -105,8 +106,8 @@ public final class NetcdfStoreProviderTest extends TestCase { @Test public void testDecoderFromUCAR() throws IOException, DataStoreException { final StorageConnector c = new StorageConnector(createUCAR(TestData.NETCDF_2D_GEOGRAPHIC)); - try (Decoder decoder = NetcdfStoreProvider.decoder(createListeners(), c)) { - assertInstanceOf("decoder", DecoderWrapper.class, decoder); - } + final Decoder decoder = NetcdfStoreProvider.decoder(createListeners(), c); + assertInstanceOf("decoder", DecoderWrapper.class, decoder); + decoder.close(new DataStoreMock("lock")); } } diff --git a/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/GridResourceWrapper.java b/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/GridResourceWrapper.java index 3b99c91f68..e4d5c628a9 100644 --- a/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/GridResourceWrapper.java +++ b/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/GridResourceWrapper.java @@ -38,7 +38,7 @@ import org.opengis.util.GenericName; * The wrapped resource is created only when first needed. * * @author Martin Desruisseaux (Geomatys) - * @version 1.2 + * @version 1.4 * @since 1.1 */ public abstract class GridResourceWrapper implements GridCoverageResource { @@ -46,7 +46,7 @@ public abstract class GridResourceWrapper implements GridCoverageResource { * The coverage resource instance which provides the data. * This is initially {@code null} and created when first needed. */ - private GridCoverageResource source; + private volatile GridCoverageResource source; /** * Creates a new wrapper. @@ -78,12 +78,16 @@ public abstract class GridResourceWrapper implements GridCoverageResource { * @throws DataStoreException if the resource cannot be created. */ protected final GridCoverageResource source() throws DataStoreException { - synchronized (getSynchronizationLock()) { - if (source == null) { - source = createSource(); + GridCoverageResource s = source; + if (s == null) { + synchronized (getSynchronizationLock()) { + s = source; + if (s == null) { + source = s = createSource(); + } } - return source; } + return s; } /** @@ -224,18 +228,16 @@ public abstract class GridResourceWrapper implements GridCoverageResource { */ @Override public <T extends StoreEvent> void removeListener(Class<T> eventType, StoreListener<? super T> listener) { - final GridCoverageResource source; - synchronized (getSynchronizationLock()) { - source = this.source; // No need to invoke the `source()` method here. - } - if (source != null) { - source.removeListener(eventType, listener); + final GridCoverageResource s = source; // No need to invoke the `source()` method here. + if (s != null) { + s.removeListener(eventType, listener); } } /** * Closes the data store associated to the resource, then discards the resource. * This method does not verify if the data store is still used by other resources. + * This method can be invoked asynchronously for interrupting a long reading process. * * @throws DataStoreException if an error occurred while closing the data store. */ diff --git a/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/csv/package-info.java b/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/csv/package-info.java index 38cc6c369e..4277b0abcd 100644 --- a/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/csv/package-info.java +++ b/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/csv/package-info.java @@ -53,7 +53,7 @@ * </ul> * * @author Martin Desruisseaux (Geomatys) - * @version 1.3 + * @version 1.4 * @since 0.7 */ package org.apache.sis.internal.storage.csv; diff --git a/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/esri/AsciiGridStore.java b/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/esri/AsciiGridStore.java index 12c31f45d5..d9c1b684ce 100644 --- a/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/esri/AsciiGridStore.java +++ b/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/esri/AsciiGridStore.java @@ -140,7 +140,7 @@ import org.apache.sis.util.resources.Errors; * which is usually the case given how inefficient this format is. * * @author Martin Desruisseaux (Geomatys) - * @version 1.3 + * @version 1.4 * @since 1.2 */ class AsciiGridStore extends RasterStore { @@ -176,7 +176,7 @@ class AsciiGridStore extends RasterStore { * Note that a null value does not necessarily means that the store is closed, because * it may have finished to read fully the {@linkplain #coverage}. */ - private CharactersView input; + private volatile CharactersView input; /** * The {@code NCOLS} and {@code NROWS} attributes read from the header. @@ -345,6 +345,7 @@ cellsize: if (value != null) { /** * Returns the error message for an exception or log record. + * Invoke only in contexts where {@link #input} is known to be non-null. * * @param rk {@link Errors.Keys#IllegalValueForProperty_2} or {@link Errors.Keys#MissingValueForProperty_2}. * @param key key of the header property which was requested. @@ -539,21 +540,27 @@ cellsize: if (value != null) { /** * Closes this data store and releases any underlying resources. + * This method can be invoked asynchronously for interrupting a long reading process. * * @throws DataStoreException if an error occurred while closing this data store. */ @Override - public synchronized void close() throws DataStoreException { - listeners.close(); // Should never fail. - final CharactersView view = input; - input = null; // Cleared first in case of failure. - coverage = null; - gridGeometry = null; - super.close(); // Clear more fields. Never fail. - if (view != null) try { - view.input.channel.close(); + public void close() throws DataStoreException { + try { + listeners.close(); // Should never fail. + final CharactersView view = input; + if (view != null) { + view.input.channel.close(); + } } catch (IOException e) { throw new DataStoreException(e); + } finally { + synchronized (this) { + super.close(); // Clear more fields. Never fail. + gridGeometry = null; + coverage = null; + input = null; + } } } diff --git a/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/esri/RasterStore.java b/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/esri/RasterStore.java index c0fa37220f..fae34dca7e 100644 --- a/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/esri/RasterStore.java +++ b/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/esri/RasterStore.java @@ -508,7 +508,7 @@ abstract class RasterStore extends PRJDataStore implements GridCoverageResource /** * Closes this data store and releases any underlying resources. - * Shall be overridden by subclasses in a synchronized method. + * Shall be overridden by subclasses inside a synchronized block. * * @throws DataStoreException if an error occurred while closing this data store. */ diff --git a/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/esri/RawRasterStore.java b/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/esri/RawRasterStore.java index 9ffac1a493..60fca66fef 100644 --- a/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/esri/RawRasterStore.java +++ b/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/esri/RawRasterStore.java @@ -61,7 +61,7 @@ import static org.apache.sis.internal.util.Numerics.wholeDiv; * * @author Johann Sorel (Geomatys) * @author Martin Desruisseaux (Geomatys) - * @version 1.3 + * @version 1.4 * @since 1.2 */ final class RawRasterStore extends RasterStore { @@ -170,7 +170,7 @@ final class RawRasterStore extends RasterStore { /** * The object to use for reading data, or {@code null} if the channel has been closed. */ - private ChannelDataInput input; + private volatile ChannelDataInput input; /** * Helper method for reading a rectangular region from the {@linkplain #input} stream. @@ -244,6 +244,7 @@ final class RawRasterStore extends RasterStore { */ @Override public synchronized List<SampleDimension> getSampleDimensions() throws DataStoreException { + final ChannelDataInput input = this.input; List<SampleDimension> sampleDimensions = super.getSampleDimensions(); if (sampleDimensions == null) try { if (reader == null) { @@ -337,6 +338,7 @@ final class RawRasterStore extends RasterStore { */ private void readHeader() throws IOException, DataStoreException { assert Thread.holdsLock(this); + final ChannelDataInput input = this.input; if (input == null) { throw new DataStoreClosedException(canNotRead()); } @@ -541,20 +543,26 @@ final class RawRasterStore extends RasterStore { /** * Closes this data store and releases any underlying resources. + * This method can be invoked asynchronously for interrupting a long reading process. * * @throws DataStoreException if an error occurred while closing this data store. */ @Override - public synchronized void close() throws DataStoreException { - listeners.close(); // Should never fail. - final ChannelDataInput in = input; - input = null; // Cleared first in case of failure. - reader = null; - super.close(); // Clear more fields. Never fail. - if (in != null) try { - in.channel.close(); - } catch (IOException e) { - throw new DataStoreException(e); + public void close() throws DataStoreException { + try { + listeners.close(); // Should never fail. + final ChannelDataInput input = this.input; + if (input != null) try { + input.channel.close(); + } catch (IOException e) { + throw new DataStoreException(e); + } + } finally { + synchronized (this) { + input = null; // Cleared first in case of failure. + reader = null; + super.close(); // Clear more fields. Never fail. + } } } } diff --git a/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/esri/WritableStore.java b/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/esri/WritableStore.java index aeb7117f5d..b4a2b1e001 100644 --- a/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/esri/WritableStore.java +++ b/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/esri/WritableStore.java @@ -289,6 +289,9 @@ final class WritableStore extends AsciiGridStore implements WritableGridCoverage /** * Closes this data store and releases any underlying resources. + * If a read or write operation is in progress in another thread, + * then this method blocks until that operation completed. + * This restriction is for avoiding data lost. * * @throws DataStoreException if an error occurred while closing this data store. */ diff --git a/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/folder/ConcurrentCloser.java b/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/folder/ConcurrentCloser.java new file mode 100644 index 0000000000..15e59f86dc --- /dev/null +++ b/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/folder/ConcurrentCloser.java @@ -0,0 +1,113 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.sis.internal.storage.folder; + +import java.util.Collection; +import java.util.concurrent.Future; +import java.util.concurrent.Callable; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.ExecutionException; +import org.apache.sis.storage.Resource; +import org.apache.sis.storage.DataStore; +import org.apache.sis.storage.DataStoreException; +import org.apache.sis.internal.storage.StoreResource; +import org.apache.sis.internal.system.CommonExecutor; + + +/** + * Helper class for closing concurrently a collection of data stores. + * + * @author Martin Desruisseaux (Geomatys) + * @version 1.4 + * + * @param <R> type of resource to close. + * + * @since 1.4 + */ +public abstract class ConcurrentCloser<R> { + /** + * A closer for a collection of resources which may be data stores. + * This closer does not check for {@link StoreResource} instances. + */ + public static final ConcurrentCloser<Resource> RESOURCES = new ConcurrentCloser<>() { + @Override protected Callable<?> closer(final Resource r) { + if (r instanceof DataStore) { + final DataStore ds = (DataStore) r; + return () -> { + ds.close(); + return null; + }; + } else return null; + } + }; + + /** + * Creates a new closer. + */ + protected ConcurrentCloser() { + } + + /** + * Creates a task to be invoked in a background thread for closing the given resource. + * The return value of the callable will be ignored. + * + * @param resource the resource to close. + * @return the task for closing the given resource, or {@code null} if none. + */ + protected abstract Callable<?> closer(R resource); + + /** + * Closes concurrently all the given resources. + * + * @param resources the resource to close. + * @throws DataStoreException if at least one error occurred while closing a resource. + */ + public final void closeAll(final Collection<? extends R> resources) throws DataStoreException { + final ExecutorService executor = CommonExecutor.instance(); + final Future<?>[] results = new Future<?>[resources.size()]; + int n = 0; + for (final R r : resources) { + final Callable<?> c = closer(r); + if (c != null) { + results[n++] = executor.submit(c); + } + } + /* + * Wait for all tasks to complete and collect + * the exceptions that are thrown, if any. + */ + DataStoreException failure = null; + for (int i=0; i<n; i++) { + try { + results[i].get(); + } catch (InterruptedException | ExecutionException ex) { + Throwable cause = ex.getCause(); + if (cause == null) cause = ex; + if (failure != null) { + failure.addSuppressed(cause); + } else if (cause instanceof DataStoreException) { + failure = (DataStoreException) cause; + } else { + failure = new DataStoreException(cause); + } + } + } + if (failure != null) { + throw failure; + } + } +} diff --git a/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/folder/Store.java b/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/folder/Store.java index f0d7d09bb1..0e1d24c903 100644 --- a/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/folder/Store.java +++ b/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/folder/Store.java @@ -76,7 +76,7 @@ import org.apache.sis.internal.storage.Resources; * * @author Johann Sorel (Geomatys) * @author Martin Desruisseaux (Geomatys) - * @version 1.3 + * @version 1.4 * @since 0.8 */ class Store extends DataStore implements StoreResource, UnstructuredAggregate, DirectoryStream.Filter<Path> { @@ -274,6 +274,7 @@ class Store extends DataStore implements StoreResource, UnstructuredAggregate, D mb.addResourceScope(ScopeCode.COLLECTION, Resources.formatInternational(Resources.Keys.DirectoryContent_1, getDisplayName())); mb.addLanguage(locale, MetadataBuilder.Scope.RESOURCE); mb.addEncoding(encoding, MetadataBuilder.Scope.RESOURCE); + final GenericName identifier = identifier(null); String name = null; if (identifier != null) { name = identifier.toString(); @@ -436,28 +437,23 @@ class Store extends DataStore implements StoreResource, UnstructuredAggregate, D /** * Closes all children resources. + * This method can be invoked asynchronously for interrupting a long reading process + * if the children stores also support asynchronous close operations. */ @Override - public synchronized void close() throws DataStoreException { - listeners.close(); // Should never fail. - final Collection<Resource> resources = components; - if (resources != null) { - components = null; // Clear first in case of failure. - DataStoreException failure = null; - for (final Resource r : resources) { - if (r instanceof DataStore) try { - ((DataStore) r).close(); - } catch (DataStoreException ex) { - if (failure == null) { - failure = ex; - } else { - failure.addSuppressed(ex); - } - } - } - if (failure != null) { - throw failure; - } + public void close() throws DataStoreException { + listeners.close(); // Should never fail. + final Collection<Resource> resources; + synchronized (this) { + resources = components; + components = List.of(); + identifier = null; + metadata = null; + structuredView = null; + children.clear(); + } + if (resources != null && !resources.isEmpty()) { + ConcurrentCloser.RESOURCES.closeAll(resources); } } } diff --git a/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/image/WorldFileStore.java b/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/image/WorldFileStore.java index 3915013420..948f1a83bb 100644 --- a/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/image/WorldFileStore.java +++ b/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/image/WorldFileStore.java @@ -111,7 +111,7 @@ import org.apache.sis.setup.OptionKey; * is known to support only one image per file. * * @author Martin Desruisseaux (Geomatys) - * @version 1.3 + * @version 1.4 * @since 1.2 */ public class WorldFileStore extends PRJDataStore { @@ -166,7 +166,7 @@ public class WorldFileStore extends PRJDataStore { * * @see #reader() */ - private ImageReader reader; + private volatile ImageReader reader; /** * The object to close when {@code WorldFileStore} is closed. It may be a different object than @@ -286,6 +286,7 @@ public class WorldFileStore extends PRJDataStore { * does not support the locale, the reader's default locale will be used. */ private void configureReader() { + final ImageReader reader = this.reader; try { reader.setLocale(listeners.getLocale()); } catch (IllegalArgumentException e) { @@ -432,6 +433,7 @@ loop: for (int convention=0;; convention++) { * @return the requested names, or an empty array if none or unknown. */ public String[] getImageFormat(final boolean asMimeType) { + final ImageReader reader = this.reader; if (reader != null) { final ImageReaderSpi provider = reader.getOriginatingProvider(); if (provider != null) { @@ -804,34 +806,38 @@ loop: for (int convention=0;; convention++) { /** * Closes this data store and releases any underlying resources. + * If a read operation is in progress, it will be aborted. * * @throws DataStoreException if an error occurred while closing this data store. */ @Override - public synchronized void close() throws DataStoreException { + public void close() throws DataStoreException { listeners.close(); // Should never fail. final ImageReader codec = reader; - final Closeable stream = toClose; - reader = null; - toClose = null; - metadata = null; - components = null; - gridGeometry = null; - try { - Object input = null; - if (codec != null) { - input = codec.getInput(); - codec.setInput(null); - codec.dispose(); - if (input instanceof AutoCloseable) { - ((AutoCloseable) input).close(); + if (codec != null) codec.abort(); + synchronized (this) { + final Closeable stream = toClose; + reader = null; + toClose = null; + metadata = null; + components = null; + gridGeometry = null; + try { + Object input = null; + if (codec != null) { + input = codec.getInput(); + codec.reset(); + codec.dispose(); + if (input instanceof AutoCloseable) { + ((AutoCloseable) input).close(); + } } + if (stream != null && stream != input) { + stream.close(); + } + } catch (Exception e) { + throw new DataStoreException(e); } - if (stream != null && stream != input) { - stream.close(); - } - } catch (Exception e) { - throw new DataStoreException(e); } } } diff --git a/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/image/WritableStore.java b/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/image/WritableStore.java index 60b69bebf7..b1ec9ff290 100644 --- a/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/image/WritableStore.java +++ b/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/image/WritableStore.java @@ -488,6 +488,9 @@ writeCoeffs: for (int i=0;; i++) { /** * Closes this data store and releases any underlying resources. + * If a read or write operation is in progress in another thread, + * then this method blocks until that operation completed. + * This restriction is for avoiding data lost. * * @throws DataStoreException if an error occurred while closing this data store. */ diff --git a/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/io/FileCacheByteChannel.java b/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/io/FileCacheByteChannel.java index 5fc3577808..c1d86edff0 100644 --- a/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/io/FileCacheByteChannel.java +++ b/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/io/FileCacheByteChannel.java @@ -273,7 +273,7 @@ public abstract class FileCacheByteChannel extends ByteRangeChannel { * @see #openConnection(long, long) * @see #abort(InputStream) */ - private Connection connection; + private volatile Connection connection; /** * An optional filter to apply on the input stream opened for a connections. @@ -747,17 +747,18 @@ public abstract class FileCacheByteChannel extends ByteRangeChannel { private long drainAndAbort() throws IOException { assert Thread.holdsLock(this); long count = 0; - final InputStream input = connection.input; - for (int c; (c = input.available()) > 0;) { + final Connection c = connection; + final InputStream input = c.input; + for (int r; (r = input.available()) > 0;) { final ByteBuffer buffer = transfer(); buffer.clear(); - if (c < BUFFER_SIZE) buffer.limit(c); + if (r < BUFFER_SIZE) buffer.limit(r); final int n = input.read(buffer.array(), 0, buffer.limit()); if (n < 0) break; cache(buffer.limit(n)); count += n; } - if (abort(connection)) { + if (abort(c)) { connection = null; } return count; @@ -803,19 +804,23 @@ public abstract class FileCacheByteChannel extends ByteRangeChannel { /** * Closes this channel and releases resources. + * This method can be invoked asynchronously for interrupting a long reading process. * * @throws IOException if an error occurred while closing the channel. */ @Override - public synchronized void close() throws IOException { + public void close() throws IOException { final Connection c = connection; - connection = null; - transfer = null; - idleHandler = null; try (file) { if (c != null && !abort(c)) { c.input.close(); } + } finally { + synchronized (this) { + transfer = null; + idleHandler = null; + connection = null; + } } } diff --git a/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/wkt/Store.java b/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/wkt/Store.java index ae7ec95337..9baaaf1774 100644 --- a/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/wkt/Store.java +++ b/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/wkt/Store.java @@ -47,7 +47,7 @@ import org.apache.sis.util.CharSequences; * the file containing WKT definition is the main file, not an auxiliary file.</div> * * @author Martin Desruisseaux (Geomatys) - * @version 1.3 + * @version 1.4 * @since 0.7 */ final class Store extends URIDataStore { @@ -61,7 +61,7 @@ final class Store extends URIDataStore { /** * The reader, set by the constructor and cleared when no longer needed. */ - private Reader source; + private volatile Reader source; /** * The locale for {@link org.opengis.util.InternationalString} localization @@ -117,7 +117,6 @@ final class Store extends URIDataStore { private void parse() throws DataStoreException { final Reader in = source; if (in != null) try { - source = null; // Cleared first in case of error. final String wkt; try { char[] buffer = new char[FirstKeywordPeek.READ_AHEAD_LIMIT]; @@ -134,6 +133,7 @@ final class Store extends URIDataStore { } wkt = String.valueOf(buffer, 0, length); } finally { + source = null; in.close(); } /* @@ -193,19 +193,24 @@ final class Store extends URIDataStore { /** * Closes this data store and releases any underlying resources. + * This method can be invoked asynchronously for interrupting a long reading process. * * @throws DataStoreException if an error occurred while closing this data store. */ @Override - public synchronized void close() throws DataStoreException { - listeners.close(); // Should never fail. - final Reader s = source; - source = null; // Cleared first in case of failure. - objects.clear(); - if (s != null) try { - s.close(); + public void close() throws DataStoreException { + try { + listeners.close(); // Should never fail. + final Reader s = source; + if (s != null) s.close(); } catch (IOException e) { throw new DataStoreException(e); + } finally { + synchronized (this) { + objects.clear(); + metadata = null; + source = null; + } } } } diff --git a/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/xml/Store.java b/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/xml/Store.java index d0c4643404..c8d0135aa0 100644 --- a/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/xml/Store.java +++ b/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/xml/Store.java @@ -56,14 +56,14 @@ import org.apache.sis.setup.OptionKey; * The above list may be extended in any future SIS version. * * @author Martin Desruisseaux (Geomatys) - * @version 1.3 + * @version 1.4 * @since 0.4 */ final class Store extends URIDataStore implements Filter { /** * The input stream or reader, set by the constructor and cleared when no longer needed. */ - private StreamSource source; + private volatile StreamSource source; /** * The unmarshalled object, initialized only when first needed. @@ -151,11 +151,11 @@ final class Store extends URIDataStore implements Filter { private void unmarshal() throws DataStoreException { final StreamSource s = source; final Closeable in = input(s); - source = null; // Cleared first in case of error. if (in != null) try { try { object = XML.unmarshal(s, properties()); } finally { + source = null; in.close(); } } catch (JAXBException | IOException e) { @@ -217,19 +217,23 @@ final class Store extends URIDataStore implements Filter { /** * Closes this data store and releases any underlying resources. + * This method can be invoked asynchronously for interrupting a long reading process. * * @throws DataStoreException if an error occurred while closing this data store. */ @Override - public synchronized void close() throws DataStoreException { - listeners.close(); // Should never fail. - object = null; - final Closeable in = input(source); - source = null; // Cleared first in case of failure. - if (in != null) try { - in.close(); + public void close() throws DataStoreException { + try { + listeners.close(); // Should never fail. + final Closeable in = input(source); + if (in != null) in.close(); } catch (IOException e) { throw new DataStoreException(e); + } finally { + synchronized (this) { + object = null; + source = null; + } } } } diff --git a/storage/sis-storage/src/main/java/org/apache/sis/storage/DataStore.java b/storage/sis-storage/src/main/java/org/apache/sis/storage/DataStore.java index 22ac8baa6c..338cefde18 100644 --- a/storage/sis-storage/src/main/java/org/apache/sis/storage/DataStore.java +++ b/storage/sis-storage/src/main/java/org/apache/sis/storage/DataStore.java @@ -528,6 +528,11 @@ public abstract class DataStore implements Resource, Localized, AutoCloseable { * Closes this data store and releases any underlying resources. * A {@link CloseEvent} is sent to listeners before the data store is closed. * + * <p>If this method is invoked asynchronously while a read operation is in progress in another thread, + * then the behavior is implementation dependent. Some implementations will interrupt the read process, + * for example with an {@link java.nio.channels.AsynchronousCloseException}. This is useful if the data + * store was downloading a large file from a network connection.</p> + * * <h4>Note for implementers</h4> * Implementations should invoke {@code listeners.close()} on their first line * for sending notification to all listeners before the data store is actually diff --git a/storage/sis-storage/src/main/java/org/apache/sis/storage/event/StoreListeners.java b/storage/sis-storage/src/main/java/org/apache/sis/storage/event/StoreListeners.java index 132133dcb2..ffd79c718a 100644 --- a/storage/sis-storage/src/main/java/org/apache/sis/storage/event/StoreListeners.java +++ b/storage/sis-storage/src/main/java/org/apache/sis/storage/event/StoreListeners.java @@ -953,11 +953,14 @@ public class StoreListeners implements Localized { } catch (ExecutionException ex) { canNotNotify("close", ex); } - listeners = null; /* - * No need to cleanup `cascadedListeners`. It does not hurt (those listeners practically - * become no-op) and the objects are probably going to be garbage collected soon anyway. + * This `StoreListeners` may not be garbage-collected immediately if the data store has been closed + * asynchronously. So clearing the following fields may help to garbage-collect some more resources. */ + synchronized (this) { + cascadedListeners = null; + listeners = null; + } } /** diff --git a/storage/sis-xmlstore/src/main/java/org/apache/sis/internal/storage/gpx/Store.java b/storage/sis-xmlstore/src/main/java/org/apache/sis/internal/storage/gpx/Store.java index e3115b6993..9fba9cb8f4 100644 --- a/storage/sis-xmlstore/src/main/java/org/apache/sis/internal/storage/gpx/Store.java +++ b/storage/sis-xmlstore/src/main/java/org/apache/sis/internal/storage/gpx/Store.java @@ -238,6 +238,7 @@ public class Store extends StaxDataStore implements FeatureSet { /** * Closes only the reader, without closing this store. * This method may be invoked before write operation. + * It must be invoked inside a synchronized block. */ final void closeReader() throws Exception { final Reader r = reader; @@ -255,6 +256,8 @@ public class Store extends StaxDataStore implements FeatureSet { @Override public synchronized void close() throws DataStoreException { listeners.close(); // Should never fail. + version = null; + metadata = null; try { closeReader(); } catch (Exception e) { diff --git a/storage/sis-xmlstore/src/main/java/org/apache/sis/internal/storage/xml/stream/StaxDataStore.java b/storage/sis-xmlstore/src/main/java/org/apache/sis/internal/storage/xml/stream/StaxDataStore.java index 49dd9e19ff..e57fb2701c 100644 --- a/storage/sis-xmlstore/src/main/java/org/apache/sis/internal/storage/xml/stream/StaxDataStore.java +++ b/storage/sis-xmlstore/src/main/java/org/apache/sis/internal/storage/xml/stream/StaxDataStore.java @@ -61,7 +61,7 @@ import org.apache.sis.storage.UnsupportedStorageException; * * @author Johann Sorel (Geomatys) * @author Martin Desruisseaux (Geomatys) - * @version 1.3 + * @version 1.4 * @since 0.8 */ public abstract class StaxDataStore extends URIDataStore { @@ -122,7 +122,7 @@ public abstract class StaxDataStore extends URIDataStore { * stream or channel opened for that path. * * <p>We keep this reference as long as possible in order to use {@link #mark()} and {@link #reset()} - * instead of creating new streams for re-reading the data. If we cannot reset the stream but can + * instead of creating new streams for re-reading the data. If we cannot reset the stream but can * create a new one, then this field will become a reference to the new stream. This change should be * done only in last resort, when there is no way to reuse the existing stream. This is because the * streams created by {@link ChannelFactory#inputStream(String, StoreListeners)} are not of the same @@ -130,7 +130,7 @@ public abstract class StaxDataStore extends URIDataStore { * * @see #close() */ - private AutoCloseable stream; + private volatile AutoCloseable stream; /** * Position of the first byte to read in the {@linkplain #stream}, or a negative value if unknown. @@ -604,16 +604,21 @@ public abstract class StaxDataStore extends URIDataStore { * @throws DataStoreException if an error occurred while closing the input or output stream. */ @Override - public synchronized void close() throws DataStoreException { - final AutoCloseable s = stream; - stream = null; - storage = null; - inputFactory = null; - outputFactory = null; - if (s != null) try { - s.close(); + public void close() throws DataStoreException { + try { + final AutoCloseable s = stream; + if (s != null) s.close(); + } catch (DataStoreException e) { + throw e; } catch (Exception e) { throw new DataStoreException(e); + } finally { + synchronized (this) { + outputFactory = null; + inputFactory = null; + storage = null; + stream = null; + } } } }