From 5f1da6956c1eaa4ff3d38e14b3ef3516d55f2bdd Mon Sep 17 00:00:00 2001 From: Sean Parker Date: Wed, 29 Jul 2020 15:53:54 +0100 Subject: [PATCH 01/17] Consistant naming in function comments --- workers/unity/Packages/io.improbable.worker.sdk/CIO.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/workers/unity/Packages/io.improbable.worker.sdk/CIO.cs b/workers/unity/Packages/io.improbable.worker.sdk/CIO.cs index dfabb649d9..4639c69908 100644 --- a/workers/unity/Packages/io.improbable.worker.sdk/CIO.cs +++ b/workers/unity/Packages/io.improbable.worker.sdk/CIO.cs @@ -142,7 +142,7 @@ public enum OpenMode * Returns the actual number of bytes read. This may be less than the given length iff the stream * has less data than the requested amount. * - * Returns -1 on error. Call StreamGetLastError() to get the associated error message. + * Returns -1 on error. Call StreamGetLastError to get the associated error message. */ [DllImport(Constants.WorkerLibrary, CallingConvention = CallingConvention.Cdecl, EntryPoint = "Io_Stream_Peek")] @@ -155,7 +155,7 @@ public enum OpenMode * has advanced. This may be less than the given length iff the stream has less data than the * requested amount. * - * Returns -1 on error. Call StreamGetLastError() to get the associated error message. + * Returns -1 on error. Call StreamGetLastError to get the associated error message. */ [DllImport(Constants.WorkerLibrary, CallingConvention = CallingConvention.Cdecl, EntryPoint = "Io_Stream_Ignore")] From 86b472e0270957c13d8a3fc78168f5cef54ba495 Mon Sep 17 00:00:00 2001 From: Sean Parker Date: Wed, 29 Jul 2020 15:54:10 +0100 Subject: [PATCH 02/17] Add IOStream native wrapper --- .../io.improbable.worker.sdk/IOStream.cs | 111 ++++++++++++++++++ .../io.improbable.worker.sdk/IOStream.cs.meta | 3 + 2 files changed, 114 insertions(+) create mode 100644 workers/unity/Packages/io.improbable.worker.sdk/IOStream.cs create mode 100644 workers/unity/Packages/io.improbable.worker.sdk/IOStream.cs.meta diff --git a/workers/unity/Packages/io.improbable.worker.sdk/IOStream.cs b/workers/unity/Packages/io.improbable.worker.sdk/IOStream.cs new file mode 100644 index 0000000000..dbea548e1f --- /dev/null +++ b/workers/unity/Packages/io.improbable.worker.sdk/IOStream.cs @@ -0,0 +1,111 @@ +using System; +using System.IO; +using System.Runtime.InteropServices; +using Improbable.Worker.CInterop.Internal; + +namespace Packages.io.improbable.worker.sdk +{ + public unsafe class IOStream : IDisposable + { + private readonly CIO.StreamHandle stream; + + private IOStream(CIO.StreamHandle stream) + { + this.stream = stream; + } + + /// + public void Dispose() + { + stream.Dispose(); + } + + public static IOStream CreateRingBufferStream(uint capacity) + { + return new IOStream(CIO.CreateRingBufferStream(capacity)); + } + + public static IOStream CreateFileStream(string fileName) + { + fixed (byte* fileNameBytes = ApiInterop.ToUtf8Cstr(fileName)) + { + return new IOStream(CIO.CreateFileStream(fileNameBytes, CIO.OpenMode.OpenModeDefault)); + } + } + + public void Write(byte[] data) + { + // Maybe use StreamGetRemainingWriteCapacityBytes to prevent attempting to write to the stream if + // sizeof(data) > remaining capacity + + var remainingCapacity = CIO.StreamGetRemainingWriteCapacityBytes(stream); + if (remainingCapacity < data.Length) + { + throw new IOException("Not enough stream capacity to write data."); + } + + fixed (byte* dataToWrite = data) + { + var bytesWritten = CIO.StreamWrite(stream, dataToWrite, 1); + + if (bytesWritten == -1) + { + // The Stream write failed, check the error message + var rawError = CIO.StreamGetLastError(stream); + throw new IOException(ApiInterop.FromUtf8Cstr(rawError)); + } + + if (bytesWritten != data.Length) + { + // Check that the number of bytes we tried to write was the number of bytes written + throw new IOException("Failed to write all data to stream."); + } + } + } + + public byte[] Read(uint bytesToRead) + { + var b = new byte[bytesToRead]; + var c = GCHandle.Alloc(b, GCHandleType.Pinned); + var bytesRead = CIO.StreamRead(stream, (byte*) c.AddrOfPinnedObject(), bytesToRead); + + if (bytesRead == -1) + { + var rawError = CIO.StreamGetLastError(stream); + throw new Exception(ApiInterop.FromUtf8Cstr(rawError)); + } + + if (bytesRead != bytesToRead) + { + throw new Exception($"Failed to read {bytesToRead} bytes, only read ${bytesRead}."); + } + + return c.Target as byte[]; + } + + public byte[] Peek(uint bytes) + { + var b = new byte[bytes]; + var c = GCHandle.Alloc(b, GCHandleType.Pinned); + var bytesPeeked = CIO.StreamPeek(stream, (byte*) c.AddrOfPinnedObject(), bytes); + + if (bytesPeeked == -1) + { + var rawError = CIO.StreamGetLastError(stream); + throw new Exception(ApiInterop.FromUtf8Cstr(rawError)); + } + + if (bytesPeeked != bytes) + { + throw new Exception($"Failed to read {bytes} bytes, only read ${bytes}."); + } + + return c.Target as byte[]; + } + + public void Ignore(uint bytesToIgnore) + { + CIO.StreamIgnore(stream, bytesToIgnore); + } + } +} diff --git a/workers/unity/Packages/io.improbable.worker.sdk/IOStream.cs.meta b/workers/unity/Packages/io.improbable.worker.sdk/IOStream.cs.meta new file mode 100644 index 0000000000..61e529facb --- /dev/null +++ b/workers/unity/Packages/io.improbable.worker.sdk/IOStream.cs.meta @@ -0,0 +1,3 @@ +fileFormatVersion: 2 +guid: b0e0fbfec8ff4c5e99a8b505c9ccb705 +timeCreated: 1595944475 \ No newline at end of file From 8ede688736442e32836f35cb7ffa7f9ae715cd3a Mon Sep 17 00:00:00 2001 From: Sean Parker Date: Wed, 29 Jul 2020 15:54:20 +0100 Subject: [PATCH 03/17] Add IOStorage native wrapper --- .../io.improbable.worker.sdk/IOStorage.cs | 28 +++++++++++++++++++ .../IOStorage.cs.meta | 3 ++ 2 files changed, 31 insertions(+) create mode 100644 workers/unity/Packages/io.improbable.worker.sdk/IOStorage.cs create mode 100644 workers/unity/Packages/io.improbable.worker.sdk/IOStorage.cs.meta diff --git a/workers/unity/Packages/io.improbable.worker.sdk/IOStorage.cs b/workers/unity/Packages/io.improbable.worker.sdk/IOStorage.cs new file mode 100644 index 0000000000..fb8cd3d7c6 --- /dev/null +++ b/workers/unity/Packages/io.improbable.worker.sdk/IOStorage.cs @@ -0,0 +1,28 @@ +using System; +using System.Runtime.InteropServices; +using Improbable.Worker.CInterop.Internal; +using Packages.io.improbable.worker.sdk; + +namespace Improbable.Worker.CInterop +{ + public class IOStorage : IDisposable + { + private readonly CIO.StorageHandle storage; + + public IOStorage() + { + storage = CIO.StorageCreate(); + } + + /// + public void Dispose() + { + storage.Dispose(); + } + + public void Clear() + { + CIO.StorageClear(storage); + } + } +} diff --git a/workers/unity/Packages/io.improbable.worker.sdk/IOStorage.cs.meta b/workers/unity/Packages/io.improbable.worker.sdk/IOStorage.cs.meta new file mode 100644 index 0000000000..e9e14df0a1 --- /dev/null +++ b/workers/unity/Packages/io.improbable.worker.sdk/IOStorage.cs.meta @@ -0,0 +1,3 @@ +fileFormatVersion: 2 +guid: e50ee44109544e7d8d9d06fe8c1243f2 +timeCreated: 1595864556 \ No newline at end of file From 5ab2acfd0dd40a3153c06b5c460b9b4b55b1591b Mon Sep 17 00:00:00 2001 From: Sean Parker Date: Wed, 29 Jul 2020 16:42:09 +0100 Subject: [PATCH 04/17] Fix naming of Storage/Stream handles in params --- .../Packages/io.improbable.worker.sdk/CIO.cs | 24 +++++++++---------- 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/workers/unity/Packages/io.improbable.worker.sdk/CIO.cs b/workers/unity/Packages/io.improbable.worker.sdk/CIO.cs index 4639c69908..5fef75ff2c 100644 --- a/workers/unity/Packages/io.improbable.worker.sdk/CIO.cs +++ b/workers/unity/Packages/io.improbable.worker.sdk/CIO.cs @@ -10,7 +10,7 @@ namespace Improbable.Worker.CInterop.Internal { internal unsafe class CIO { - public class Storage : CptrHandle + public class StorageHandle : CptrHandle { protected override bool ReleaseHandle() { @@ -19,7 +19,7 @@ protected override bool ReleaseHandle() } } - public class Stream : CptrHandle + public class StreamHandle : CptrHandle { protected override bool ReleaseHandle() { @@ -45,7 +45,7 @@ public enum OpenMode */ [DllImport(Constants.WorkerLibrary, CallingConvention = CallingConvention.Cdecl, EntryPoint = "Io_Storage_Create")] - public static extern Storage StorageCreate(); + public static extern StorageHandle StorageCreate(); /* Destroys the trace storage. */ [DllImport(Constants.WorkerLibrary, CallingConvention = CallingConvention.Cdecl, @@ -61,7 +61,7 @@ public enum OpenMode */ [DllImport(Constants.WorkerLibrary, CallingConvention = CallingConvention.Cdecl, EntryPoint = "Io_Storage_Clear")] - public static extern void StorageClear(IntPtr storage); + public static extern void StorageClear(StorageHandle storage); /** * Creates an I/O stream implemented as a ring buffer. @@ -77,7 +77,7 @@ public enum OpenMode */ [DllImport(Constants.WorkerLibrary, CallingConvention = CallingConvention.Cdecl, EntryPoint = "Io_CreateRingBufferStream")] - public static extern Stream CreateRingBufferStream(Uint32 capacityBytes); + public static extern StreamHandle CreateRingBufferStream(Uint32 capacityBytes); /** * Creates an I/O stream implemented as a read/write file. @@ -94,7 +94,7 @@ public enum OpenMode */ [DllImport(Constants.WorkerLibrary, CallingConvention = CallingConvention.Cdecl, EntryPoint = "Io_CreateFileStream")] - public static extern Stream CreateFileStream(Char* filename, OpenMode openMode); + public static extern StreamHandle CreateFileStream(Char* filename, OpenMode openMode); /* Destroys the I/O stream. */ [DllImport(Constants.WorkerLibrary, CallingConvention = CallingConvention.Cdecl, @@ -111,7 +111,7 @@ public enum OpenMode */ [DllImport(Constants.WorkerLibrary, CallingConvention = CallingConvention.Cdecl, EntryPoint = "Io_Stream_Write")] - public static extern Int64 StreamWrite(Stream stream, Uint8* bytes, Uint32 length); + public static extern Int64 StreamWrite(StreamHandle stream, Uint8* bytes, Uint32 length); /** * Gets the remaining write capacity in bytes. @@ -121,7 +121,7 @@ public enum OpenMode */ [DllImport(Constants.WorkerLibrary, CallingConvention = CallingConvention.Cdecl, EntryPoint = "Io_Stream_GetRemainingWriteCapacityBytes")] - public static extern Uint32 StreamGetRemainingWriteCapacityBytes(Stream stream); + public static extern Uint32 StreamGetRemainingWriteCapacityBytes(StreamHandle stream); /** * Reads as much of the stream's data as possible into the given buffer. @@ -133,7 +133,7 @@ public enum OpenMode */ [DllImport(Constants.WorkerLibrary, CallingConvention = CallingConvention.Cdecl, EntryPoint = "Io_Stream_Read")] - public static extern Uint64 StreamRead(Stream stream, Uint8* bytes, Uint32 length); + public static extern Uint64 StreamRead(StreamHandle stream, Uint8* bytes, Uint32 length); /** * Reads as much of the stream's data as possible into the given buffer without advancing the read @@ -146,7 +146,7 @@ public enum OpenMode */ [DllImport(Constants.WorkerLibrary, CallingConvention = CallingConvention.Cdecl, EntryPoint = "Io_Stream_Peek")] - public static extern Int64 StreamPeek(Stream stream, Uint8* bytes, Uint32 length); + public static extern Int64 StreamPeek(StreamHandle stream, Uint8* bytes, Uint32 length); /** * Extracts the given number of bytes from the stream and discards them. @@ -159,7 +159,7 @@ public enum OpenMode */ [DllImport(Constants.WorkerLibrary, CallingConvention = CallingConvention.Cdecl, EntryPoint = "Io_Stream_Ignore")] - public static extern Int64 StreamIgnore(Stream stream, Uint32 length); + public static extern Int64 StreamIgnore(StreamHandle stream, Uint32 length); /** * Returns the last error which occurred during an API call on this stream. Returns nullptr if no @@ -167,6 +167,6 @@ public enum OpenMode */ [DllImport(Constants.WorkerLibrary, CallingConvention = CallingConvention.Cdecl, EntryPoint = "Io_Stream_GetLastError")] - public static extern Char* StreamGetLastError(Stream stream); + public static extern Char* StreamGetLastError(StreamHandle stream); } } From d24a3e8de5c014ee32378e45524cd42b8e237938 Mon Sep 17 00:00:00 2001 From: Sean Parker Date: Wed, 29 Jul 2020 16:53:11 +0100 Subject: [PATCH 05/17] Consistant naming for Stream/Storage handles --- .../Packages/io.improbable.worker.sdk/CEventTrace.cs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/workers/unity/Packages/io.improbable.worker.sdk/CEventTrace.cs b/workers/unity/Packages/io.improbable.worker.sdk/CEventTrace.cs index 337d7e32f3..b35e12f852 100644 --- a/workers/unity/Packages/io.improbable.worker.sdk/CEventTrace.cs +++ b/workers/unity/Packages/io.improbable.worker.sdk/CEventTrace.cs @@ -278,7 +278,7 @@ public struct EventTracerParameters */ [DllImport(Constants.WorkerLibrary, CallingConvention = CallingConvention.Cdecl, EntryPoint = "Trace_Item_Create")] - public static extern Item* ItemCreate(CIO.Storage storage, Item* item); + public static extern Item* ItemCreate(CIO.StorageHandle storage, Item* item); /** * Returns a pointer to a thread-local trace item. @@ -320,7 +320,7 @@ public struct EventTracerParameters */ [DllImport(Constants.WorkerLibrary, CallingConvention = CallingConvention.Cdecl, EntryPoint = "Trace_SerializeItemToStream")] - public static extern Int8 SerializeItemToStream(CIO.Stream stream, Item* item, Uint32 size); + public static extern Int8 SerializeItemToStream(CIO.StreamHandle stream, Item* item, Uint32 size); /** * Get the serialized size, in bytes, of the next serialized trace item to be read from the stream. @@ -332,7 +332,7 @@ public struct EventTracerParameters */ [DllImport(Constants.WorkerLibrary, CallingConvention = CallingConvention.Cdecl, EntryPoint = "Trace_GetNextSerializedItemSize")] - public static extern Uint32 GetNextSerializedItemSize(CIO.Stream stream); + public static extern Uint32 GetNextSerializedItemSize(CIO.StreamHandle stream); /** * Deserialize the next trace item from the stream. @@ -356,7 +356,7 @@ public struct EventTracerParameters */ [DllImport(Constants.WorkerLibrary, CallingConvention = CallingConvention.Cdecl, EntryPoint = "Trace_DeserializeItemFromStream")] - public static extern Int8 DeserializeItemFromStream(CIO.Stream stream, Item* item, Uint32 size); + public static extern Int8 DeserializeItemFromStream(CIO.StreamHandle stream, Item* item, Uint32 size); /** * Returns the last error which occurred during a trace API method call. Returns nullptr if no From fbd7af708c099719ca60c0377b304b808a343f0c Mon Sep 17 00:00:00 2001 From: Sean Parker Date: Thu, 30 Jul 2020 16:52:40 +0100 Subject: [PATCH 06/17] Code cleanup --- workers/unity/Packages/io.improbable.worker.sdk/IOStorage.cs | 2 -- workers/unity/Packages/io.improbable.worker.sdk/IOStream.cs | 2 +- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/workers/unity/Packages/io.improbable.worker.sdk/IOStorage.cs b/workers/unity/Packages/io.improbable.worker.sdk/IOStorage.cs index fb8cd3d7c6..ec2bacb4d6 100644 --- a/workers/unity/Packages/io.improbable.worker.sdk/IOStorage.cs +++ b/workers/unity/Packages/io.improbable.worker.sdk/IOStorage.cs @@ -1,7 +1,5 @@ using System; -using System.Runtime.InteropServices; using Improbable.Worker.CInterop.Internal; -using Packages.io.improbable.worker.sdk; namespace Improbable.Worker.CInterop { diff --git a/workers/unity/Packages/io.improbable.worker.sdk/IOStream.cs b/workers/unity/Packages/io.improbable.worker.sdk/IOStream.cs index dbea548e1f..73fc00e6c9 100644 --- a/workers/unity/Packages/io.improbable.worker.sdk/IOStream.cs +++ b/workers/unity/Packages/io.improbable.worker.sdk/IOStream.cs @@ -3,7 +3,7 @@ using System.Runtime.InteropServices; using Improbable.Worker.CInterop.Internal; -namespace Packages.io.improbable.worker.sdk +namespace Improbable.Worker.CInterop { public unsafe class IOStream : IDisposable { From 2b1fec92541a957e778eabec5ee43c2bdcec0ec6 Mon Sep 17 00:00:00 2001 From: Sean Parker Date: Thu, 30 Jul 2020 16:52:58 +0100 Subject: [PATCH 07/17] Fix return type for CIO StreamRead --- workers/unity/Packages/io.improbable.worker.sdk/CIO.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/workers/unity/Packages/io.improbable.worker.sdk/CIO.cs b/workers/unity/Packages/io.improbable.worker.sdk/CIO.cs index 5fef75ff2c..2bc19c2605 100644 --- a/workers/unity/Packages/io.improbable.worker.sdk/CIO.cs +++ b/workers/unity/Packages/io.improbable.worker.sdk/CIO.cs @@ -133,7 +133,7 @@ public enum OpenMode */ [DllImport(Constants.WorkerLibrary, CallingConvention = CallingConvention.Cdecl, EntryPoint = "Io_Stream_Read")] - public static extern Uint64 StreamRead(StreamHandle stream, Uint8* bytes, Uint32 length); + public static extern Int64 StreamRead(StreamHandle stream, Uint8* bytes, Uint32 length); /** * Reads as much of the stream's data as possible into the given buffer without advancing the read From 152c85af4920674a1327a06ee13d4f7666b07760 Mon Sep 17 00:00:00 2001 From: Sean Parker Date: Thu, 30 Jul 2020 17:20:03 +0100 Subject: [PATCH 08/17] Variable naming in IOStream, add error handling --- .../io.improbable.worker.sdk/IOStream.cs | 40 +++++++++++-------- 1 file changed, 24 insertions(+), 16 deletions(-) diff --git a/workers/unity/Packages/io.improbable.worker.sdk/IOStream.cs b/workers/unity/Packages/io.improbable.worker.sdk/IOStream.cs index 73fc00e6c9..a703c6e00c 100644 --- a/workers/unity/Packages/io.improbable.worker.sdk/IOStream.cs +++ b/workers/unity/Packages/io.improbable.worker.sdk/IOStream.cs @@ -35,9 +35,6 @@ public static IOStream CreateFileStream(string fileName) public void Write(byte[] data) { - // Maybe use StreamGetRemainingWriteCapacityBytes to prevent attempting to write to the stream if - // sizeof(data) > remaining capacity - var remainingCapacity = CIO.StreamGetRemainingWriteCapacityBytes(stream); if (remainingCapacity < data.Length) { @@ -65,47 +62,58 @@ public void Write(byte[] data) public byte[] Read(uint bytesToRead) { - var b = new byte[bytesToRead]; - var c = GCHandle.Alloc(b, GCHandleType.Pinned); - var bytesRead = CIO.StreamRead(stream, (byte*) c.AddrOfPinnedObject(), bytesToRead); + var streamData = new byte[bytesToRead]; + var dataHandle = GCHandle.Alloc(streamData, GCHandleType.Pinned); + var bytesRead = CIO.StreamRead(stream, (byte*) dataHandle.AddrOfPinnedObject(), bytesToRead); if (bytesRead == -1) { var rawError = CIO.StreamGetLastError(stream); - throw new Exception(ApiInterop.FromUtf8Cstr(rawError)); + throw new IOException(ApiInterop.FromUtf8Cstr(rawError)); } if (bytesRead != bytesToRead) { - throw new Exception($"Failed to read {bytesToRead} bytes, only read ${bytesRead}."); + throw new IOException($"Failed to read {bytesToRead} bytes, only read {bytesRead}."); } - return c.Target as byte[]; + return dataHandle.Target as byte[]; } public byte[] Peek(uint bytes) { - var b = new byte[bytes]; - var c = GCHandle.Alloc(b, GCHandleType.Pinned); - var bytesPeeked = CIO.StreamPeek(stream, (byte*) c.AddrOfPinnedObject(), bytes); + var streamData = new byte[bytes]; + var dataHandle = GCHandle.Alloc(streamData, GCHandleType.Pinned); + var bytesPeeked = CIO.StreamPeek(stream, (byte*) dataHandle.AddrOfPinnedObject(), bytes); if (bytesPeeked == -1) { var rawError = CIO.StreamGetLastError(stream); - throw new Exception(ApiInterop.FromUtf8Cstr(rawError)); + throw new IOException(ApiInterop.FromUtf8Cstr(rawError)); } if (bytesPeeked != bytes) { - throw new Exception($"Failed to read {bytes} bytes, only read ${bytes}."); + throw new IOException($"Failed to read {bytes} bytes, only read {bytes}."); } - return c.Target as byte[]; + return dataHandle.Target as byte[]; } public void Ignore(uint bytesToIgnore) { - CIO.StreamIgnore(stream, bytesToIgnore); + var bytesIgnored = CIO.StreamIgnore(stream, bytesToIgnore); + + if (bytesIgnored == -1) + { + var rawError = CIO.StreamGetLastError(stream); + throw new IOException(ApiInterop.FromUtf8Cstr(rawError)); + } + + if (bytesIgnored != bytesToIgnore) + { + throw new IOException($"Failed to ignore {bytesToIgnore} bytes, only ignored {bytesToIgnore}."); + } } } } From bbe294e3e37a324ce1204da6c67a78bc32e2b4b4 Mon Sep 17 00:00:00 2001 From: Sean Parker Date: Thu, 30 Jul 2020 17:22:43 +0100 Subject: [PATCH 09/17] Add changelog entry --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 17fce49b94..dc28ff6058 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,7 @@ ### Internal - Added C# bindings for C Event Tracing API. [#1440](https://github.com/spatialos/gdk-for-unity/pull/1440) +- Added native classes for IO operations in Event Tracing API. [#1444](https://github.com/spatialos/gdk-for-unity/pull/1444) ## `0.3.9` - 2020-07-24 From 445390cf016f1987623ccd89692a25e18ca6dd9d Mon Sep 17 00:00:00 2001 From: Sean Parker Date: Mon, 3 Aug 2020 11:02:02 +0100 Subject: [PATCH 10/17] Add GC suppress finialize call to dispose method --- workers/unity/Packages/io.improbable.worker.sdk/IOStorage.cs | 1 + workers/unity/Packages/io.improbable.worker.sdk/IOStream.cs | 1 + 2 files changed, 2 insertions(+) diff --git a/workers/unity/Packages/io.improbable.worker.sdk/IOStorage.cs b/workers/unity/Packages/io.improbable.worker.sdk/IOStorage.cs index ec2bacb4d6..986f83daa1 100644 --- a/workers/unity/Packages/io.improbable.worker.sdk/IOStorage.cs +++ b/workers/unity/Packages/io.improbable.worker.sdk/IOStorage.cs @@ -16,6 +16,7 @@ public IOStorage() public void Dispose() { storage.Dispose(); + GC.SuppressFinalize(this); } public void Clear() diff --git a/workers/unity/Packages/io.improbable.worker.sdk/IOStream.cs b/workers/unity/Packages/io.improbable.worker.sdk/IOStream.cs index a703c6e00c..b6129752f1 100644 --- a/workers/unity/Packages/io.improbable.worker.sdk/IOStream.cs +++ b/workers/unity/Packages/io.improbable.worker.sdk/IOStream.cs @@ -18,6 +18,7 @@ private IOStream(CIO.StreamHandle stream) public void Dispose() { stream.Dispose(); + GC.SuppressFinalize(this); } public static IOStream CreateRingBufferStream(uint capacity) From 297ebf5d66dd0a45918e3dedcc4fd4ceedf6d70d Mon Sep 17 00:00:00 2001 From: Sean Parker Date: Mon, 3 Aug 2020 14:25:45 +0100 Subject: [PATCH 11/17] Change from throwing errors to return byte amounts Removed reference to internal namespace enum --- .../io.improbable.worker.sdk/IOStream.cs | 101 ++++++++++-------- 1 file changed, 58 insertions(+), 43 deletions(-) diff --git a/workers/unity/Packages/io.improbable.worker.sdk/IOStream.cs b/workers/unity/Packages/io.improbable.worker.sdk/IOStream.cs index b6129752f1..2056f383c3 100644 --- a/workers/unity/Packages/io.improbable.worker.sdk/IOStream.cs +++ b/workers/unity/Packages/io.improbable.worker.sdk/IOStream.cs @@ -5,9 +5,15 @@ namespace Improbable.Worker.CInterop { - public unsafe class IOStream : IDisposable + public enum OpenMode { - private readonly CIO.StreamHandle stream; + /* Opens the stream in the default mode. */ + OpenModeDefault = 0x00, + } + + public sealed unsafe class IOStream : IDisposable + { + public readonly CIO.StreamHandle stream; private IOStream(CIO.StreamHandle stream) { @@ -26,15 +32,15 @@ public static IOStream CreateRingBufferStream(uint capacity) return new IOStream(CIO.CreateRingBufferStream(capacity)); } - public static IOStream CreateFileStream(string fileName) + public static IOStream CreateFileStream(string fileName, OpenMode openMode) { fixed (byte* fileNameBytes = ApiInterop.ToUtf8Cstr(fileName)) { - return new IOStream(CIO.CreateFileStream(fileNameBytes, CIO.OpenMode.OpenModeDefault)); + return new IOStream(CIO.CreateFileStream(fileNameBytes, (CIO.OpenMode) openMode)); } } - public void Write(byte[] data) + public long Write(byte[] data) { var remainingCapacity = CIO.StreamGetRemainingWriteCapacityBytes(stream); if (remainingCapacity < data.Length) @@ -42,79 +48,88 @@ public void Write(byte[] data) throw new IOException("Not enough stream capacity to write data."); } + var bytesWritten = 0L; fixed (byte* dataToWrite = data) { - var bytesWritten = CIO.StreamWrite(stream, dataToWrite, 1); - - if (bytesWritten == -1) - { - // The Stream write failed, check the error message - var rawError = CIO.StreamGetLastError(stream); - throw new IOException(ApiInterop.FromUtf8Cstr(rawError)); - } - - if (bytesWritten != data.Length) - { - // Check that the number of bytes we tried to write was the number of bytes written - throw new IOException("Failed to write all data to stream."); - } + bytesWritten = CIO.StreamWrite(stream, dataToWrite, 1); } + + if (bytesWritten != -1) + { + return bytesWritten; + } + + var rawError = CIO.StreamGetLastError(stream); + throw new IOException(ApiInterop.FromUtf8Cstr(rawError)); } - public byte[] Read(uint bytesToRead) + public (byte[], long) Read(uint bytesToRead) { var streamData = new byte[bytesToRead]; - var dataHandle = GCHandle.Alloc(streamData, GCHandleType.Pinned); - var bytesRead = CIO.StreamRead(stream, (byte*) dataHandle.AddrOfPinnedObject(), bytesToRead); - if (bytesRead == -1) + var bytesRead = 0L; + fixed (byte* streamDataPointer = streamData) + { + bytesRead = CIO.StreamRead(stream, streamDataPointer, bytesToRead); + } + + if (bytesRead != -1) { - var rawError = CIO.StreamGetLastError(stream); - throw new IOException(ApiInterop.FromUtf8Cstr(rawError)); + return (streamData, bytesRead); } - if (bytesRead != bytesToRead) + var rawError = CIO.StreamGetLastError(stream); + throw new IOException(ApiInterop.FromUtf8Cstr(rawError)); + } + + public long Read(byte[] streamData) + { + var bytesToRead = (uint) streamData.Length; + var bytesRead = 0L; + fixed (byte* streamDataPointer = streamData) { - throw new IOException($"Failed to read {bytesToRead} bytes, only read {bytesRead}."); + bytesRead = CIO.StreamRead(stream, streamDataPointer, bytesToRead); } - return dataHandle.Target as byte[]; + if (bytesRead != -1) + { + return bytesRead; + } + + var rawError = CIO.StreamGetLastError(stream); + throw new IOException(ApiInterop.FromUtf8Cstr(rawError)); } public byte[] Peek(uint bytes) { var streamData = new byte[bytes]; - var dataHandle = GCHandle.Alloc(streamData, GCHandleType.Pinned); - var bytesPeeked = CIO.StreamPeek(stream, (byte*) dataHandle.AddrOfPinnedObject(), bytes); - if (bytesPeeked == -1) + var bytesPeeked = 0L; + fixed (byte* streamDataPointer = streamData) { - var rawError = CIO.StreamGetLastError(stream); - throw new IOException(ApiInterop.FromUtf8Cstr(rawError)); + bytesPeeked = CIO.StreamPeek(stream, streamDataPointer, bytes); } - if (bytesPeeked != bytes) + if (bytesPeeked != -1) { - throw new IOException($"Failed to read {bytes} bytes, only read {bytes}."); + return streamData; } - return dataHandle.Target as byte[]; + var rawError = CIO.StreamGetLastError(stream); + throw new IOException(ApiInterop.FromUtf8Cstr(rawError)); } public void Ignore(uint bytesToIgnore) { var bytesIgnored = CIO.StreamIgnore(stream, bytesToIgnore); - if (bytesIgnored == -1) + if (bytesIgnored != -1) { - var rawError = CIO.StreamGetLastError(stream); - throw new IOException(ApiInterop.FromUtf8Cstr(rawError)); + return; } - if (bytesIgnored != bytesToIgnore) - { - throw new IOException($"Failed to ignore {bytesToIgnore} bytes, only ignored {bytesToIgnore}."); - } + var rawError = CIO.StreamGetLastError(stream); + throw new IOException(ApiInterop.FromUtf8Cstr(rawError)); } } } From fe76a7f011bd8bd94dbe66e36efa135a47db0008 Mon Sep 17 00:00:00 2001 From: Sean Parker Date: Mon, 3 Aug 2020 14:52:25 +0100 Subject: [PATCH 12/17] Fix IO stream scope for StreamHandle --- workers/unity/Packages/io.improbable.worker.sdk/IOStream.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/workers/unity/Packages/io.improbable.worker.sdk/IOStream.cs b/workers/unity/Packages/io.improbable.worker.sdk/IOStream.cs index 2056f383c3..c750f071fa 100644 --- a/workers/unity/Packages/io.improbable.worker.sdk/IOStream.cs +++ b/workers/unity/Packages/io.improbable.worker.sdk/IOStream.cs @@ -13,7 +13,7 @@ public enum OpenMode public sealed unsafe class IOStream : IDisposable { - public readonly CIO.StreamHandle stream; + private readonly CIO.StreamHandle stream; private IOStream(CIO.StreamHandle stream) { From fe6c03b0d6756b15516d854df6366784720aae1e Mon Sep 17 00:00:00 2001 From: Sean Parker Date: Mon, 3 Aug 2020 15:08:12 +0100 Subject: [PATCH 13/17] Add sealed to IOStorage class --- workers/unity/Packages/io.improbable.worker.sdk/IOStorage.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/workers/unity/Packages/io.improbable.worker.sdk/IOStorage.cs b/workers/unity/Packages/io.improbable.worker.sdk/IOStorage.cs index 986f83daa1..a64e3026c8 100644 --- a/workers/unity/Packages/io.improbable.worker.sdk/IOStorage.cs +++ b/workers/unity/Packages/io.improbable.worker.sdk/IOStorage.cs @@ -3,7 +3,7 @@ namespace Improbable.Worker.CInterop { - public class IOStorage : IDisposable + public sealed class IOStorage : IDisposable { private readonly CIO.StorageHandle storage; From 56a343a163e66a97368e5c5e3004d9c19888fa92 Mon Sep 17 00:00:00 2001 From: Sean Parker Date: Mon, 3 Aug 2020 17:10:51 +0100 Subject: [PATCH 14/17] Remove GC SuppressFinalize call --- workers/unity/Packages/io.improbable.worker.sdk/IOStorage.cs | 1 - workers/unity/Packages/io.improbable.worker.sdk/IOStream.cs | 1 - 2 files changed, 2 deletions(-) diff --git a/workers/unity/Packages/io.improbable.worker.sdk/IOStorage.cs b/workers/unity/Packages/io.improbable.worker.sdk/IOStorage.cs index a64e3026c8..e2f58019f0 100644 --- a/workers/unity/Packages/io.improbable.worker.sdk/IOStorage.cs +++ b/workers/unity/Packages/io.improbable.worker.sdk/IOStorage.cs @@ -16,7 +16,6 @@ public IOStorage() public void Dispose() { storage.Dispose(); - GC.SuppressFinalize(this); } public void Clear() diff --git a/workers/unity/Packages/io.improbable.worker.sdk/IOStream.cs b/workers/unity/Packages/io.improbable.worker.sdk/IOStream.cs index c750f071fa..7645678c46 100644 --- a/workers/unity/Packages/io.improbable.worker.sdk/IOStream.cs +++ b/workers/unity/Packages/io.improbable.worker.sdk/IOStream.cs @@ -24,7 +24,6 @@ private IOStream(CIO.StreamHandle stream) public void Dispose() { stream.Dispose(); - GC.SuppressFinalize(this); } public static IOStream CreateRingBufferStream(uint capacity) From 423dc22d63dd847c5d15b1533b4b50361d5dd34d Mon Sep 17 00:00:00 2001 From: Sean Parker Date: Mon, 3 Aug 2020 17:49:25 +0100 Subject: [PATCH 15/17] Switch from tuple return to out var for IO read --- .../unity/Packages/io.improbable.worker.sdk/IOStream.cs | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/workers/unity/Packages/io.improbable.worker.sdk/IOStream.cs b/workers/unity/Packages/io.improbable.worker.sdk/IOStream.cs index 7645678c46..fe9beca601 100644 --- a/workers/unity/Packages/io.improbable.worker.sdk/IOStream.cs +++ b/workers/unity/Packages/io.improbable.worker.sdk/IOStream.cs @@ -1,6 +1,5 @@ using System; using System.IO; -using System.Runtime.InteropServices; using Improbable.Worker.CInterop.Internal; namespace Improbable.Worker.CInterop @@ -62,9 +61,9 @@ public long Write(byte[] data) throw new IOException(ApiInterop.FromUtf8Cstr(rawError)); } - public (byte[], long) Read(uint bytesToRead) + public long Read(uint bytesToRead, out byte[] streamData) { - var streamData = new byte[bytesToRead]; + streamData = new byte[bytesToRead]; var bytesRead = 0L; fixed (byte* streamDataPointer = streamData) @@ -74,7 +73,7 @@ public long Write(byte[] data) if (bytesRead != -1) { - return (streamData, bytesRead); + return bytesRead; } var rawError = CIO.StreamGetLastError(stream); From b7409d6a0d0139604201d56ca1da0ed7b80ed4d9 Mon Sep 17 00:00:00 2001 From: Sean Parker Date: Tue, 4 Aug 2020 13:55:09 +0100 Subject: [PATCH 16/17] Add error handling to check if stream closed --- .../io.improbable.worker.sdk/IOStream.cs | 37 +++++++++++++++---- 1 file changed, 30 insertions(+), 7 deletions(-) diff --git a/workers/unity/Packages/io.improbable.worker.sdk/IOStream.cs b/workers/unity/Packages/io.improbable.worker.sdk/IOStream.cs index fe9beca601..d8c618ba97 100644 --- a/workers/unity/Packages/io.improbable.worker.sdk/IOStream.cs +++ b/workers/unity/Packages/io.improbable.worker.sdk/IOStream.cs @@ -40,10 +40,12 @@ public static IOStream CreateFileStream(string fileName, OpenMode openMode) public long Write(byte[] data) { + CheckIfStreamClosed(); + var remainingCapacity = CIO.StreamGetRemainingWriteCapacityBytes(stream); if (remainingCapacity < data.Length) { - throw new IOException("Not enough stream capacity to write data."); + throw new NotSupportedException("Not enough stream capacity to write data."); } var bytesWritten = 0L; @@ -63,6 +65,8 @@ public long Write(byte[] data) public long Read(uint bytesToRead, out byte[] streamData) { + CheckIfStreamClosed(); + streamData = new byte[bytesToRead]; var bytesRead = 0L; @@ -82,6 +86,8 @@ public long Read(uint bytesToRead, out byte[] streamData) public long Read(byte[] streamData) { + CheckIfStreamClosed(); + var bytesToRead = (uint) streamData.Length; var bytesRead = 0L; fixed (byte* streamDataPointer = streamData) @@ -98,36 +104,53 @@ public long Read(byte[] streamData) throw new IOException(ApiInterop.FromUtf8Cstr(rawError)); } - public byte[] Peek(uint bytes) + public long Peek(uint bytesToPeek, out byte[] streamData) { - var streamData = new byte[bytes]; + CheckIfStreamClosed(); + + streamData = new byte[bytesToPeek]; var bytesPeeked = 0L; fixed (byte* streamDataPointer = streamData) { - bytesPeeked = CIO.StreamPeek(stream, streamDataPointer, bytes); + bytesPeeked = CIO.StreamPeek(stream, streamDataPointer, bytesToPeek); } if (bytesPeeked != -1) { - return streamData; + return bytesPeeked; } var rawError = CIO.StreamGetLastError(stream); throw new IOException(ApiInterop.FromUtf8Cstr(rawError)); } - public void Ignore(uint bytesToIgnore) + public long Ignore(uint bytesToIgnore) { + CheckIfStreamClosed(); + var bytesIgnored = CIO.StreamIgnore(stream, bytesToIgnore); if (bytesIgnored != -1) { - return; + return bytesIgnored; } var rawError = CIO.StreamGetLastError(stream); throw new IOException(ApiInterop.FromUtf8Cstr(rawError)); } + + public uint GetRemainingCapacity() + { + return CIO.StreamGetRemainingWriteCapacityBytes(stream); + } + + private void CheckIfStreamClosed() + { + if (stream.IsClosed) + { + throw new ObjectDisposedException("Cannot access a disposed object."); + } + } } } From bc9adbdd150b8ce4c4d1129884701c95047ca54a Mon Sep 17 00:00:00 2001 From: Sean Parker Date: Tue, 4 Aug 2020 14:14:40 +0100 Subject: [PATCH 17/17] Clarify function name if stream is closed --- .../Packages/io.improbable.worker.sdk/IOStream.cs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/workers/unity/Packages/io.improbable.worker.sdk/IOStream.cs b/workers/unity/Packages/io.improbable.worker.sdk/IOStream.cs index d8c618ba97..5a48eada96 100644 --- a/workers/unity/Packages/io.improbable.worker.sdk/IOStream.cs +++ b/workers/unity/Packages/io.improbable.worker.sdk/IOStream.cs @@ -40,7 +40,7 @@ public static IOStream CreateFileStream(string fileName, OpenMode openMode) public long Write(byte[] data) { - CheckIfStreamClosed(); + ThrowIfStreamClosed(); var remainingCapacity = CIO.StreamGetRemainingWriteCapacityBytes(stream); if (remainingCapacity < data.Length) @@ -65,7 +65,7 @@ public long Write(byte[] data) public long Read(uint bytesToRead, out byte[] streamData) { - CheckIfStreamClosed(); + ThrowIfStreamClosed(); streamData = new byte[bytesToRead]; @@ -86,7 +86,7 @@ public long Read(uint bytesToRead, out byte[] streamData) public long Read(byte[] streamData) { - CheckIfStreamClosed(); + ThrowIfStreamClosed(); var bytesToRead = (uint) streamData.Length; var bytesRead = 0L; @@ -106,7 +106,7 @@ public long Read(byte[] streamData) public long Peek(uint bytesToPeek, out byte[] streamData) { - CheckIfStreamClosed(); + ThrowIfStreamClosed(); streamData = new byte[bytesToPeek]; @@ -127,7 +127,7 @@ public long Peek(uint bytesToPeek, out byte[] streamData) public long Ignore(uint bytesToIgnore) { - CheckIfStreamClosed(); + ThrowIfStreamClosed(); var bytesIgnored = CIO.StreamIgnore(stream, bytesToIgnore); @@ -145,7 +145,7 @@ public uint GetRemainingCapacity() return CIO.StreamGetRemainingWriteCapacityBytes(stream); } - private void CheckIfStreamClosed() + private void ThrowIfStreamClosed() { if (stream.IsClosed) {