Make IFolder interface inherently IDisposable.

Fix up implementations to ensure they dispose any stream they acquire, and ensure the constructor will not leave a stream open if it fails. Dispose folders when unmounting them in GlobalFileSystem.

OpenRA.Game/FileSystem/BagFile.cs

@@ -19,7 +19,7 @@ using OpenRA.Primitives;
 
 namespace OpenRA.FileSystem
 {
-	public sealed class BagFile : IFolder, IDisposable
+	public sealed class BagFile : IFolder
 	{
 		static readonly uint[] Nothing = { };
 
@@ -37,20 +37,16 @@ namespace OpenRA.FileSystem
 			// For example: audio.bag requires the audio.idx file
 			var indexFilename = Path.ChangeExtension(filename, ".idx");
 
-			s = GlobalFileSystem.Open(filename);
-
 			// Build the index and dispose the stream, it is no longer needed after this
 			List<IdxEntry> entries;
 			using (var indexStream = GlobalFileSystem.Open(indexFilename))
-			{
+				entries = new IdxReader(indexStream).Entries;
-				var reader = new IdxReader(indexStream);
-
-				entries = reader.Entries;
-			}
 
 			index = entries.ToDictionaryWithConflictLog(x => x.Hash,
 				"{0} (bag format)".F(filename),
 				null, x => "(offs={0}, len={1})".F(x.Offset, x.Length));
+
+			s = GlobalFileSystem.Open(filename);
 		}
 
 		public int Priority { get { return 1000 + bagFilePriority; } }
@@ -185,8 +181,7 @@ namespace OpenRA.FileSystem
 
 		public void Dispose()
 		{
-			if (s != null)
+			s.Dispose();
-				s.Dispose();
 		}
 	}
 }

OpenRA.Game/FileSystem/BigFile.cs

@@ -15,37 +15,45 @@ using System.Linq;
 
 namespace OpenRA.FileSystem
 {
-	public class BigFile : IFolder
+	public sealed class BigFile : IFolder
 	{
 		public string Name { get; private set; }
 		public int Priority { get; private set; }
 		readonly Dictionary<string, Entry> entries = new Dictionary<string, Entry>();
+		readonly Stream s;
 
 		public BigFile(string filename, int priority)
 		{
 			Name = filename;
 			Priority = priority;
 
-			var s = GlobalFileSystem.Open(filename);
+			s = GlobalFileSystem.Open(filename);
-
+			try
-			if (s.ReadASCII(4) != "BIGF")
-				throw new InvalidDataException("Header is not BIGF");
-
-			// Total archive size.
-			s.ReadUInt32();
-
-			var entryCount = s.ReadUInt32();
-			if (BitConverter.IsLittleEndian)
-				entryCount = int2.Swap(entryCount);
-
-			// First entry offset? This is apparently bogus for EA's .big files
-			// and we don't have to try seeking there since the entries typically start next in EA's .big files.
-			s.ReadUInt32();
-
-			for (var i = 0; i < entryCount; i++)
 			{
-				var entry = new Entry(s);
+				if (s.ReadASCII(4) != "BIGF")
-				entries.Add(entry.Path, entry);
+					throw new InvalidDataException("Header is not BIGF");
+
+				// Total archive size.
+				s.ReadUInt32();
+
+				var entryCount = s.ReadUInt32();
+				if (BitConverter.IsLittleEndian)
+					entryCount = int2.Swap(entryCount);
+
+				// First entry offset? This is apparently bogus for EA's .big files
+				// and we don't have to try seeking there since the entries typically start next in EA's .big files.
+				s.ReadUInt32();
+
+				for (var i = 0; i < entryCount; i++)
+				{
+					var entry = new Entry(s);
+					entries.Add(entry.Path, entry);
+				}
+			}
+			catch
+			{
+				Dispose();
+				throw;
 			}
 		}
 
@@ -107,5 +115,10 @@ namespace OpenRA.FileSystem
 		{
 			throw new NotImplementedException();
 		}
+
+		public void Dispose()
+		{
+			s.Dispose();
+		}
 	}
 }

OpenRA.Game/FileSystem/D2kSoundResources.cs

@@ -14,7 +14,7 @@ using System.IO;
 
 namespace OpenRA.FileSystem
 {
-	public class D2kSoundResources : IFolder
+	public sealed class D2kSoundResources : IFolder
 	{
 		readonly Stream s;
 
@@ -30,22 +30,28 @@ namespace OpenRA.FileSystem
 			this.priority = priority;
 
 			s = GlobalFileSystem.Open(filename);
-			s.Seek(0, SeekOrigin.Begin);
+			try
-
-			filenames = new List<string>();
-
-			var headerLength = s.ReadUInt32();
-			while (s.Position < headerLength + 4)
 			{
-				var name = s.ReadASCIIZ();
+				filenames = new List<string>();
-				var offset = s.ReadUInt32();
-				var length = s.ReadUInt32();
 
-				var hash = PackageEntry.HashFilename(name, PackageHashType.Classic);
+				var headerLength = s.ReadUInt32();
-				if (!index.ContainsKey(hash))
+				while (s.Position < headerLength + 4)
-					index.Add(hash, new PackageEntry(hash, offset, length));
+				{
+					var name = s.ReadASCIIZ();
+					var offset = s.ReadUInt32();
+					var length = s.ReadUInt32();
 
-				filenames.Add(name);
+					var hash = PackageEntry.HashFilename(name, PackageHashType.Classic);
+					if (!index.ContainsKey(hash))
+						index.Add(hash, new PackageEntry(hash, offset, length));
+
+					filenames.Add(name);
+				}
+			}
+			catch
+			{
+				Dispose();
+				throw;
 			}
 		}
 
@@ -92,5 +98,10 @@ namespace OpenRA.FileSystem
 		{
 			throw new NotImplementedException("Cannot save Dune 2000 Sound Resources.");
 		}
+
+		public void Dispose()
+		{
+			s.Dispose();
+		}
 	}
 }

OpenRA.Game/FileSystem/Folder.cs

@@ -13,7 +13,7 @@ using System.IO;
 
 namespace OpenRA.FileSystem
 {
-	public class Folder : IFolder
+	public sealed class Folder : IFolder
 	{
 		readonly string path;
 		readonly int priority;
@@ -78,5 +78,7 @@ namespace OpenRA.FileSystem
 				using (var writer = new BinaryWriter(dataStream))
 					writer.Write(file.Value);
 		}
+
+		public void Dispose() { }
 	}
 }

OpenRA.Game/FileSystem/GlobalFileSystem.cs

@@ -17,7 +17,7 @@ using OpenRA.Primitives;
 
 namespace OpenRA.FileSystem
 {
-	public interface IFolder
+	public interface IFolder : IDisposable
 	{
 		Stream GetContent(string filename);
 		bool Exists(string filename);
@@ -127,6 +127,9 @@ namespace OpenRA.FileSystem
 
 		public static void UnmountAll()
 		{
+			foreach (var folder in MountedFolders)
+				folder.Dispose();
+
 			MountedFolders.Clear();
 			FolderPaths.Clear();
 			classicHashIndex = new Cache<uint, List<IFolder>>(_ => new List<IFolder>());
@@ -135,6 +138,9 @@ namespace OpenRA.FileSystem
 
 		public static bool Unmount(IFolder mount)
 		{
+			if (MountedFolders.Contains(mount))
+				mount.Dispose();
+
 			return MountedFolders.RemoveAll(f => f == mount) > 0;
 		}
 

OpenRA.Game/FileSystem/InstallShieldPackage.cs

@@ -15,7 +15,7 @@ using OpenRA.FileFormats;
 
 namespace OpenRA.FileSystem
 {
-	public class InstallShieldPackage : IFolder
+	public sealed class InstallShieldPackage : IFolder
 	{
 		readonly Dictionary<uint, PackageEntry> index = new Dictionary<uint, PackageEntry>();
 		readonly List<string> filenames;
@@ -28,38 +28,47 @@ namespace OpenRA.FileSystem
 		{
 			this.filename = filename;
 			this.priority = priority;
+
 			filenames = new List<string>();
+
 			s = GlobalFileSystem.Open(filename);
+			try
+			{
+				// Parse package header
+				var reader = new BinaryReader(s);
+				var signature = reader.ReadUInt32();
+				if (signature != 0x8C655D13)
+					throw new InvalidDataException("Not an Installshield package");
 
-			// Parse package header
+				reader.ReadBytes(8);
-			var reader = new BinaryReader(s);
+				/*var FileCount = */reader.ReadUInt16();
-			var signature = reader.ReadUInt32();
+				reader.ReadBytes(4);
-			if (signature != 0x8C655D13)
+				/*var ArchiveSize = */reader.ReadUInt32();
-				throw new InvalidDataException("Not an Installshield package");
+				reader.ReadBytes(19);
+				var tocAddress = reader.ReadInt32();
+				reader.ReadBytes(4);
+				var dirCount = reader.ReadUInt16();
 
-			reader.ReadBytes(8);
+				// Parse the directory list
-			/*var FileCount = */reader.ReadUInt16();
+				s.Seek(tocAddress, SeekOrigin.Begin);
-			reader.ReadBytes(4);
+				var tocReader = new BinaryReader(s);
-			/*var ArchiveSize = */reader.ReadUInt32();
-			reader.ReadBytes(19);
-			var tocAddress = reader.ReadInt32();
-			reader.ReadBytes(4);
-			var dirCount = reader.ReadUInt16();
 
-			// Parse the directory list
+				var fileCountInDirs = new List<uint>();
-			s.Seek(tocAddress, SeekOrigin.Begin);
-			var tocReader = new BinaryReader(s);
 
-			var fileCountInDirs = new List<uint>();
+				// Parse directories
+				for (var i = 0; i < dirCount; i++)
+					fileCountInDirs.Add(ParseDirectory(tocReader));
 
-			// Parse directories
+				// Parse files
-			for (var i = 0; i < dirCount; i++)
+				foreach (var fileCount in fileCountInDirs)
-				fileCountInDirs.Add(ParseDirectory(tocReader));
+					for (var i = 0; i < fileCount; i++)
-
+						ParseFile(reader);
-			// Parse files
+			}
-			foreach (var fileCount in fileCountInDirs)
+			catch
-				for (var i = 0; i < fileCount; i++)
+			{
-					ParseFile(reader);
+				Dispose();
+				throw;
+			}
 		}
 
 		static uint ParseDirectory(BinaryReader reader)
@@ -140,5 +149,10 @@ namespace OpenRA.FileSystem
 		{
 			throw new NotImplementedException("Cannot save InstallShieldPackages.");
 		}
+
+		public void Dispose()
+		{
+			s.Dispose();
+		}
 	}
 }

OpenRA.Game/FileSystem/MixFile.cs

@@ -17,7 +17,7 @@ using OpenRA.FileFormats;
 
 namespace OpenRA.FileSystem
 {
-	public sealed class MixFile : IFolder, IDisposable
+	public sealed class MixFile : IFolder
 	{
 		readonly Dictionary<uint, PackageEntry> index;
 		readonly long dataStart;
@@ -37,9 +37,17 @@ namespace OpenRA.FileSystem
 				File.Delete(filename);
 
 			s = File.Create(filename);
-			index = new Dictionary<uint, PackageEntry>();
+			try
-			contents.Add("local mix database.dat", new XccLocalDatabase(contents.Keys.Append("local mix database.dat")).Data());
+			{
-			Write(contents);
+				index = new Dictionary<uint, PackageEntry>();
+				contents.Add("local mix database.dat", new XccLocalDatabase(contents.Keys.Append("local mix database.dat")).Data());
+				Write(contents);
+			}
+			catch
+			{
+				Dispose();
+				throw;
+			}
 		}
 
 		public MixFile(string filename, PackageHashType type, int priority)
@@ -47,29 +55,36 @@ namespace OpenRA.FileSystem
 			this.filename = filename;
 			this.priority = priority;
 			this.type = type;
+
 			s = GlobalFileSystem.Open(filename);
-
+			try
-			// Detect format type
-			s.Seek(0, SeekOrigin.Begin);
-			var isCncMix = s.ReadUInt16() != 0;
-
-			// The C&C mix format doesn't contain any flags or encryption
-			var isEncrypted = false;
-			if (!isCncMix)
-				isEncrypted = (s.ReadUInt16() & 0x2) != 0;
-
-			List<PackageEntry> entries;
-			if (isEncrypted)
 			{
-				long unused;
+				// Detect format type
-				entries = ParseHeader(DecryptHeader(s, 4, out dataStart), 0, out unused);
+				var isCncMix = s.ReadUInt16() != 0;
-			}
-			else
-				entries = ParseHeader(s, isCncMix ? 0 : 4, out dataStart);
 
-			index = entries.ToDictionaryWithConflictLog(x => x.Hash,
+				// The C&C mix format doesn't contain any flags or encryption
-				"{0} ({1} format, Encrypted: {2}, DataStart: {3})".F(filename, isCncMix ? "C&C" : "RA/TS/RA2", isEncrypted, dataStart),
+				var isEncrypted = false;
-				null, x => "(offs={0}, len={1})".F(x.Offset, x.Length));
+				if (!isCncMix)
+					isEncrypted = (s.ReadUInt16() & 0x2) != 0;
+
+				List<PackageEntry> entries;
+				if (isEncrypted)
+				{
+					long unused;
+					entries = ParseHeader(DecryptHeader(s, 4, out dataStart), 0, out unused);
+				}
+				else
+					entries = ParseHeader(s, isCncMix ? 0 : 4, out dataStart);
+
+				index = entries.ToDictionaryWithConflictLog(x => x.Hash,
+					"{0} ({1} format, Encrypted: {2}, DataStart: {3})".F(filename, isCncMix ? "C&C" : "RA/TS/RA2", isEncrypted, dataStart),
+					null, x => "(offs={0}, len={1})".F(x.Offset, x.Length));
+			}
+			catch (Exception)
+			{
+				Dispose();
+				throw;
+			}
 		}
 
 		static List<PackageEntry> ParseHeader(Stream s, long offset, out long headerEnd)
@@ -269,8 +284,7 @@ namespace OpenRA.FileSystem
 
 		public void Dispose()
 		{
-			if (s != null)
+			s.Dispose();
-				s.Dispose();
 		}
 	}
 }

OpenRA.Game/FileSystem/Pak.cs

@@ -21,34 +21,42 @@ namespace OpenRA.FileSystem
 		public string Filename;
 	}
 
-	public class PakFile : IFolder
+	public sealed class PakFile : IFolder
 	{
-		string filename;
+		readonly string filename;
-		int priority;
+		readonly int priority;
-		Dictionary<string, Entry> index;
+		readonly Dictionary<string, Entry> index;
-		Stream stream;
+		readonly Stream stream;
 
 		public PakFile(string filename, int priority)
 		{
 			this.filename = filename;
 			this.priority = priority;
 			index = new Dictionary<string, Entry>();
+
 			stream = GlobalFileSystem.Open(filename);
-
+			try
-			index = new Dictionary<string, Entry>();
-			var offset = stream.ReadUInt32();
-			while (offset != 0)
 			{
-				var file = stream.ReadASCIIZ();
+				index = new Dictionary<string, Entry>();
-				var next = stream.ReadUInt32();
+				var offset = stream.ReadUInt32();
-				var length = (next == 0 ? (uint)stream.Length : next) - offset;
+				while (offset != 0)
+				{
+					var file = stream.ReadASCIIZ();
+					var next = stream.ReadUInt32();
+					var length = (next == 0 ? (uint)stream.Length : next) - offset;
 
-				// Ignore duplicate files
+					// Ignore duplicate files
-				if (index.ContainsKey(file))
+					if (index.ContainsKey(file))
-					continue;
+						continue;
 
-				index.Add(file, new Entry { Offset = offset, Length = length, Filename = file });
+					index.Add(file, new Entry { Offset = offset, Length = length, Filename = file });
-				offset = next;
+					offset = next;
+				}
+			}
+			catch
+			{
+				Dispose();
+				throw;
 			}
 		}
 
@@ -92,5 +100,10 @@ namespace OpenRA.FileSystem
 
 		public int Priority { get { return 1000 + priority; } }
 		public string Name { get { return filename; } }
+
+		public void Dispose()
+		{
+			stream.Dispose();
+		}
 	}
 }

OpenRA.Game/FileSystem/ZipFile.cs

@@ -17,11 +17,11 @@ using SZipFile = ICSharpCode.SharpZipLib.Zip.ZipFile;
 
 namespace OpenRA.FileSystem
 {
-	public sealed class ZipFile : IFolder, IDisposable
+	public sealed class ZipFile : IFolder
 	{
-		string filename;
+		readonly string filename;
+		readonly int priority;
 		SZipFile pkg;
-		int priority;
 
 		static ZipFile()
 		{