[C#] Thread safe reads of Double and Float values from a ByteBuffer (#5900)

* Fixed refractoring issue in reflection/generate_code.sh. Also, mv deletes the original file, so I don't need to clean it up manually in that case.

* Thread safe reads of Double and Floats from ByteBuffer

net/FlatBuffers/ByteBuffer.cs

@@ -289,11 +289,14 @@ namespace FlatBuffers
 #endif
 
 #if !UNSAFE_BYTEBUFFER
-        // Pre-allocated helper arrays for convertion.
-        private float[] floathelper = new[] { 0.0f };
-        private int[] inthelper = new[] { 0 };
-        private double[] doublehelper = new[] { 0.0 };
-        private ulong[] ulonghelper = new[] { 0UL };
+        // A conversion union where all the members are overlapping. This allows to reinterpret the bytes of one type
+        // as another, without additional copies.
+        [StructLayout(LayoutKind.Explicit)]
+        struct ConversionUnion
+        {
+          [FieldOffset(0)] public int intValue;
+          [FieldOffset(0)] public float floatValue;
+        }
 #endif // !UNSAFE_BYTEBUFFER
 
         // Helper functions for the unsafe version.
@@ -586,17 +589,18 @@ namespace FlatBuffers
         public void PutFloat(int offset, float value)
         {
             AssertOffsetAndLength(offset, sizeof(float));
-            floathelper[0] = value;
-            Buffer.BlockCopy(floathelper, 0, inthelper, 0, sizeof(float));
-            WriteLittleEndian(offset, sizeof(float), (ulong)inthelper[0]);
+            // TODO(derekbailey): use BitConvert.SingleToInt32Bits() whenever flatbuffers upgrades to a .NET version
+            // that contains it.
+            ConversionUnion union;
+            union.intValue = 0;
+            union.floatValue = value;    
+            WriteLittleEndian(offset, sizeof(float), (ulong)union.intValue);
         }
 
         public void PutDouble(int offset, double value)
         {
             AssertOffsetAndLength(offset, sizeof(double));
-            doublehelper[0] = value;
-            Buffer.BlockCopy(doublehelper, 0, ulonghelper, 0, sizeof(double));
-            WriteLittleEndian(offset, sizeof(double), ulonghelper[0]);
+            WriteLittleEndian(offset, sizeof(double), (ulong)BitConverter.DoubleToInt64Bits(value));
         }
 
 #endif // UNSAFE_BYTEBUFFER
@@ -782,19 +786,17 @@ namespace FlatBuffers
 
         public float GetFloat(int index)
         {
-            int i = (int)ReadLittleEndian(index, sizeof(float));
-            inthelper[0] = i;
-            Buffer.BlockCopy(inthelper, 0, floathelper, 0, sizeof(float));
-            return floathelper[0];
+            // TODO(derekbailey): use BitConvert.Int32BitsToSingle() whenever flatbuffers upgrades to a .NET version
+            // that contains it.
+            ConversionUnion union;
+            union.floatValue = 0;
+            union.intValue = (int)ReadLittleEndian(index, sizeof(float));
+            return union.floatValue;
         }
 
         public double GetDouble(int index)
         {
-            ulong i = ReadLittleEndian(index, sizeof(double));
-            // There's Int64BitsToDouble but it uses unsafe code internally.
-            ulonghelper[0] = i;
-            Buffer.BlockCopy(ulonghelper, 0, doublehelper, 0, sizeof(double));
-            return doublehelper[0];
+            return BitConverter.Int64BitsToDouble((long)ReadLittleEndian(index, sizeof(double)));
         }
 #endif // UNSAFE_BYTEBUFFER
 

tests/FlatBuffers.Test/ByteBufferTests.cs

@@ -608,5 +608,25 @@ namespace FlatBuffers.Test
             var data = new dummyStruct[10];
             Assert.Throws<ArgumentException>(() => uut.Put(1024, data));
         }
+
+        [FlatBuffersTestMethod]
+        public void ByteBuffer_Get_Double()
+        {
+            var uut = new ByteBuffer(1024);
+            double value = 3.14159265;
+            uut.PutDouble(900, value);
+            double getValue = uut.GetDouble(900);
+            Assert.AreEqual(value, getValue);
+        }
+
+        [FlatBuffersTestMethod]
+        public void ByteBuffer_Get_Float()
+        {
+            var uut = new ByteBuffer(1024);
+            float value = 3.14159265F;
+            uut.PutFloat(900, value);
+            double getValue = uut.GetFloat(900);
+            Assert.AreEqual(value, getValue);
+        }
     }
 }

tests/FlatBuffers.Test/FlatBuffers.Test.csproj

@@ -42,7 +42,6 @@
     </Reference>
     <Reference Include="System" />
     <Reference Include="System.Core">
-      <RequiredTargetFramework>3.5</RequiredTargetFramework>
     </Reference>
   </ItemGroup>
   <ItemGroup>

tests/FlatBuffers.Test/FlatBuffersExampleTests.cs

@@ -16,6 +16,7 @@
 
 using System.IO;
 using System.Text;
+using System.Threading;
 using MyGame.Example;
 
 namespace FlatBuffers.Test
@@ -828,5 +829,64 @@ namespace FlatBuffers.Test
             var e = MovieT.DeserializeFromBinary(fbBuffer);
             AreEqual(a, e);
         }
+
+        // For use in TestParallelAccess test case.
+        static private int _comparisons = 0;
+        static private int _failures = 0;
+        static private void KeepComparing(Monster mon, int count, float floatValue, double doubleValue)
+        {
+            int i = 0;
+            while (++i <= count)
+            {
+                Interlocked.Add(ref _comparisons, 1);
+                if(mon.Pos.Value.Test1 != doubleValue || mon.Pos.Value.Z != floatValue) {
+                    Interlocked.Add(ref _failures, 1);
+                }
+            }
+        }
+
+        [FlatBuffersTestMethod]
+        public void TestParallelAccess() {
+            // Tests that reading from a flatbuffer over multiple threads is thread-safe in regard to double and float
+            // values, since they previously were non-thread safe
+            const float floatValue = 3.141592F;
+            const double doubleValue = 1.618033988;
+
+            var fbb = new FlatBufferBuilder(1);
+            var str = fbb.CreateString("ParallelTest");
+            Monster.StartMonster(fbb);
+            Monster.AddPos(fbb, Vec3.CreateVec3(fbb, 1.0f, 2.0f, floatValue, doubleValue,
+                                                     Color.Green, (short)5, (sbyte)6));
+
+            Monster.AddName(fbb, str);
+            Monster.FinishMonsterBuffer(fbb, Monster.EndMonster(fbb));
+
+            var mon = Monster.GetRootAsMonster(fbb.DataBuffer);
+
+            var pos = mon.Pos.Value;
+            Assert.AreEqual(pos.Test1, doubleValue);
+            Assert.AreEqual(pos.Z, floatValue);
+
+            const int thread_count = 10;
+            const int reps = 1000000;
+
+            // Need to use raw Threads since Tasks are not supported in .NET 3.5
+            Thread[] threads = new Thread[thread_count];
+            for(int i = 0; i < thread_count; i++) {
+               threads[i] = new Thread(() => KeepComparing(mon, reps, floatValue, doubleValue));
+            }
+            for(int i = 0; i < thread_count; i++) {
+               threads[i].Start();
+            }
+            for(int i = 0; i < thread_count; i++) {
+               threads[i].Join();
+            }
+
+            // Make sure the threads actually did the comparisons.
+            Assert.AreEqual(thread_count * reps, _comparisons);
+
+            // Make sure we never read the values incorrectly.
+            Assert.AreEqual(0, _failures);
+        }
     }
 }

tests/FlatBuffers.Test/NetTest.sh

@@ -4,10 +4,10 @@
 mkdir dotnet_tmp
 curl -OL https://dot.net/v1/dotnet-install.sh
 chmod +x dotnet-install.sh
-./dotnet-install.sh --version 3.1.101 --install-dir dotnet_tmp
+./dotnet-install.sh --version latest --install-dir dotnet_tmp
 dotnet_tmp/dotnet new sln
 dotnet_tmp/dotnet sln add FlatBuffers.Test.csproj
-curl -OL https://dist.nuget.org/win-x86-commandline/v5.4.0/nuget.exe
+curl -OL https://dist.nuget.org/win-x86-commandline/v5.5.1/nuget.exe
 mono nuget.exe restore
 
 # Copy Test Files