diff --git a/src/main/java/dan200/computercraft/core/filesystem/ChannelWrapper.java b/src/main/java/dan200/computercraft/core/filesystem/ChannelWrapper.java new file mode 100644 index 000000000..ee4457032 --- /dev/null +++ b/src/main/java/dan200/computercraft/core/filesystem/ChannelWrapper.java @@ -0,0 +1,50 @@ +/* + * This file is part of ComputerCraft - http://www.computercraft.info + * Copyright Daniel Ratcliffe, 2011-2019. Do not distribute without permission. + * Send enquiries to dratcliffe@gmail.com + */ + +package dan200.computercraft.core.filesystem; + +import java.io.Closeable; +import java.io.IOException; +import java.nio.channels.Channel; + +/** + * Wraps some closeable object such as a buffered writer, and the underlying stream. + * + * When flushing a buffer before closing, some implementations will not close the buffer if an exception is thrown + * this causes us to release the channel, but not actually close it. This wrapper will attempt to close the wrapper (and + * so hopefully flush the channel), and then close the underlying channel. + * + * @param The type of the closeable object to write. + */ +class ChannelWrapper implements Closeable +{ + private final T wrapper; + private final Channel channel; + + ChannelWrapper( T wrapper, Channel channel ) + { + this.wrapper = wrapper; + this.channel = channel; + } + + @Override + public void close() throws IOException + { + try + { + wrapper.close(); + } + finally + { + channel.close(); + } + } + + public T get() + { + return wrapper; + } +} diff --git a/src/main/java/dan200/computercraft/core/filesystem/FileSystem.java b/src/main/java/dan200/computercraft/core/filesystem/FileSystem.java index a34dd4931..7c54bcafa 100644 --- a/src/main/java/dan200/computercraft/core/filesystem/FileSystem.java +++ b/src/main/java/dan200/computercraft/core/filesystem/FileSystem.java @@ -19,6 +19,7 @@ import java.io.IOException; import java.lang.ref.Reference; import java.lang.ref.ReferenceQueue; import java.lang.ref.WeakReference; +import java.nio.channels.Channel; import java.nio.channels.ReadableByteChannel; import java.nio.channels.WritableByteChannel; import java.nio.file.AccessDeniedException; @@ -317,7 +318,7 @@ public class FileSystem private final FileSystemWrapperMount m_wrapper = new FileSystemWrapperMount( this ); private final Map m_mounts = new HashMap<>(); - private final HashMap>, Closeable> m_openFiles = new HashMap<>(); + private final HashMap>, ChannelWrapper> m_openFiles = new HashMap<>(); private final ReferenceQueue> m_openFileQueue = new ReferenceQueue<>(); public FileSystem( String rootLabel, IMount rootMount ) throws FileSystemException @@ -661,7 +662,7 @@ public class FileSystem } } - private synchronized FileSystemWrapper openFile( @Nonnull T file ) throws FileSystemException + private synchronized FileSystemWrapper openFile( @Nonnull Channel channel, @Nonnull T file ) throws FileSystemException { synchronized( m_openFiles ) { @@ -669,12 +670,14 @@ public class FileSystem m_openFiles.size() >= ComputerCraft.maximumFilesOpen ) { IoUtil.closeQuietly( file ); + IoUtil.closeQuietly( channel ); throw new FileSystemException( "Too many files already open" ); } - FileSystemWrapper wrapper = new FileSystemWrapper<>( this, file, m_openFileQueue ); - m_openFiles.put( wrapper.self, file ); - return wrapper; + ChannelWrapper channelWrapper = new ChannelWrapper( file, channel ); + FileSystemWrapper fsWrapper = new FileSystemWrapper<>( this, channelWrapper, m_openFileQueue ); + m_openFiles.put( fsWrapper.self, channelWrapper ); + return fsWrapper; } } @@ -695,7 +698,7 @@ public class FileSystem ReadableByteChannel channel = mount.openForRead( path ); if( channel != null ) { - return openFile( open.apply( channel ) ); + return openFile( channel, open.apply( channel ) ); } return null; } @@ -709,7 +712,7 @@ public class FileSystem WritableByteChannel channel = append ? mount.openForAppend( path ) : mount.openForWrite( path ); if( channel != null ) { - return openFile( open.apply( channel ) ); + return openFile( channel, open.apply( channel ) ); } return null; } diff --git a/src/main/java/dan200/computercraft/core/filesystem/FileSystemWrapper.java b/src/main/java/dan200/computercraft/core/filesystem/FileSystemWrapper.java index d9e859c78..bbeae1146 100644 --- a/src/main/java/dan200/computercraft/core/filesystem/FileSystemWrapper.java +++ b/src/main/java/dan200/computercraft/core/filesystem/FileSystemWrapper.java @@ -15,18 +15,23 @@ import java.lang.ref.WeakReference; /** * An alternative closeable implementation that will free up resources in the filesystem. * + * The {@link FileSystem} maps weak references of this to its underlying object. If the wrapper has been disposed of + * (say, the Lua object referencing it has gone), then the wrapped object will be closed by the filesystem. + * + * Closing this will stop the filesystem tracking it, reducing the current descriptor count. + * * In an ideal world, we'd just wrap the closeable. However, as we do some {@code instanceof} checks * on the stream, it's not really possible as it'd require numerous instances. * - * @param The stream to wrap. + * @param The type of writer or channel to wrap. */ public class FileSystemWrapper implements Closeable { private final FileSystem fileSystem; - private final T closeable; + private final ChannelWrapper closeable; final WeakReference> self; - FileSystemWrapper( FileSystem fileSystem, T closeable, ReferenceQueue> queue ) + FileSystemWrapper( FileSystem fileSystem, ChannelWrapper closeable, ReferenceQueue> queue ) { this.fileSystem = fileSystem; this.closeable = closeable; @@ -43,6 +48,6 @@ public class FileSystemWrapper implements Closeable @Nonnull public T get() { - return closeable; + return closeable.get(); } } diff --git a/src/main/java/dan200/computercraft/core/lua/CobaltLuaMachine.java b/src/main/java/dan200/computercraft/core/lua/CobaltLuaMachine.java index 015862407..644c6a90b 100644 --- a/src/main/java/dan200/computercraft/core/lua/CobaltLuaMachine.java +++ b/src/main/java/dan200/computercraft/core/lua/CobaltLuaMachine.java @@ -213,7 +213,11 @@ public class CobaltLuaMachine implements ILuaMachine if( m_mainRoutine.getStatus().equals( "dead" ) ) close(); } - catch( LuaError | HardAbortError | InterruptedException e ) + catch( HardAbortError | InterruptedException e ) + { + close(); + } + catch( LuaError e ) { close(); ComputerCraft.log.warn( "Top level coroutine errored", e );