Prevent segfault when underlying repository is disposed.

4OTC · Jul 13, 2023 · 853cb88 · 853cb88
1 parent b8d3acd
commit 853cb88
Show file tree

Hide file tree

Showing 6 changed files with 75 additions and 10 deletions.
diff --git a/Directory.Packages.props b/Directory.Packages.props
@@ -1,6 +1,7 @@
 <Project>
   <ItemGroup>
 
+    <PackageVersion Include="FourOTC.DotMetrics.Codec" Version="1.1.0" />
     <PackageVersion Include="Agrona" Version="1.40.0" />
     <PackageVersion Include="InfluxDB.Collector" Version="1.2.0-dev-00029" />
     <PackageVersion Include="InfluxDB.LineProtocol" Version="1.2.0-dev-00029" />

diff --git a/DotMetrics.Codec.Test/Metrics/MappedMetricRepositoryTest.cs b/DotMetrics.Codec.Test/Metrics/MappedMetricRepositoryTest.cs
@@ -1,3 +1,4 @@
+using System.Runtime.InteropServices;
 using System.Text;
 using DotMetrics.Codec.Metrics;
 using Xunit;
@@ -21,6 +22,35 @@ public MappedMetricRepositoryTest()
         _repository = new MappedMetricRepository(_mappedFileInfo, RecordCount);
     }
 
+    [Fact]
+    public void ShouldNotSegFaultIfUnderlyingRepositoryIsDisposed()
+    {
+        IMetricCounter metricCounter = _repository.GetOrCreate(MetricOne);
+
+        metricCounter.SetValue(17);
+
+        _repository.Dispose();
+
+        metricCounter.SetValue(23);
+    }
+
+    [Fact]
+    public void ShouldCreateInDevShmOnLinux()
+    {
+        if (RuntimeInformation.IsOSPlatform(OSPlatform.Linux))
+        {
+            FileInfo fileInfo = new FileInfo($"/dev/shm/{Guid.NewGuid().ToString()}");
+            try
+            {
+                MappedMetricRepository repo = new MappedMetricRepository(fileInfo, 16);
+            }
+            finally
+            {
+                File.Delete(fileInfo.FullName);
+            }
+        }
+    }
+
     [Fact]
     public void ShouldUpdateMetricTimeOnEachPublication()
     {
@@ -38,7 +68,7 @@ public void ShouldUpdateMetricTimeOnEachPublication()
     public void ShouldNotReportMetricsUntilValueWritten()
     {
         Assert.NotNull(_repository.GetOrCreate(MetricOne));
-        
+
         _repository.Read(_receiver);
         Assert.Empty(_receiver.CapturedMetrics);
     }
@@ -48,10 +78,10 @@ public void ShouldReOpen()
     {
         _repository.GetOrCreate(MetricOne).SetValue(17);
         _repository.GetOrCreate(MetricTwo).SetValue(37);
-        
+
         _repository.Dispose();
         MappedMetricRepository repository = new MappedMetricRepository(_mappedFileInfo, RecordCount);
-        
+
         VerifyMetricValueInRepository(17, MetricOne, repository);
         VerifyMetricValueInRepository(37, MetricTwo, repository);
     }
@@ -61,7 +91,7 @@ public void ShouldGrowBackingFileWhenRecreated()
     {
         const int increasedRecordCount = 2 * RecordCount;
         MappedMetricRepository repository = new MappedMetricRepository(_mappedFileInfo, increasedRecordCount);
-        
+
         VerifyRepositoryThrowsExceptionWhenFull(increasedRecordCount, repository);
     }
 

diff --git a/DotMetrics.Codec/DotMetrics.Codec.csproj b/DotMetrics.Codec/DotMetrics.Codec.csproj
@@ -1,7 +1,7 @@
 <Project Sdk="Microsoft.NET.Sdk">
     <PropertyGroup>
         <PackageId>FourOTC.DotMetrics.Codec</PackageId>
-        <Version>1.1.0</Version>
+        <Version>1.2.0</Version>
         <PackageTags>metrics monitoring observability</PackageTags>
         <Description>
             FourOTC DotMetrics.Codec provides a serialisation format for metric values.

diff --git a/DotMetrics.Codec/Metrics/MappedMetricCounter.cs b/DotMetrics.Codec/Metrics/MappedMetricCounter.cs
@@ -10,20 +10,40 @@ public class MappedMetricCounter : IMetricCounter, IDisposable
     private readonly byte[] _valueBuffer = new byte[BitUtil.SIZE_OF_DOUBLE];
     private readonly IAtomicBuffer _buffer;
     private readonly IEpochMillisSupplier _epochMillisSupplier;
+    private readonly Func<bool> _repositoryIsDisposed;
 
     public MappedMetricCounter(
-        IAtomicBuffer buffer, 
-        IEpochMillisSupplier epochMillisSupplier)
+        IAtomicBuffer buffer,
+        IEpochMillisSupplier epochMillisSupplier,
+        Func<bool> repositoryIsDisposed)
     {
         _epochMillisSupplier = epochMillisSupplier;
+        _repositoryIsDisposed = repositoryIsDisposed;
         _buffer = buffer;
     }
 
     public void SetValue(double value)
     {
+        if (_repositoryIsDisposed())
+        {
+            return;
+        }
+
         _buffer.PutLongOrdered(BitUtil.SIZE_OF_DOUBLE, 0);
         MemoryMarshal.TryWrite(_valueBuffer, ref value);
+
+        if (_repositoryIsDisposed())
+        {
+            return;
+        }
+
         _buffer.PutBytes(0, _valueBuffer);
+
+        if (_repositoryIsDisposed())
+        {
+            return;
+        }
+
         _buffer.PutLongOrdered(BitUtil.SIZE_OF_DOUBLE, _epochMillisSupplier.EpochMs());
     }
 

diff --git a/DotMetrics.Codec/Metrics/MappedMetricRepository.cs b/DotMetrics.Codec/Metrics/MappedMetricRepository.cs
@@ -36,6 +36,8 @@ public class MappedMetricRepository : IMetricRepository, IDisposable
     private readonly uint _maxRecordCount;
     private readonly MappedByteBuffer _mappedByteBuffer;
     private readonly UnsafeBuffer _unsafeBuffer;
+    private readonly Func<bool> _repositoryIsDisposed;
+    private bool _isDisposed;
 
     public MappedMetricRepository(FileSystemInfo fileInfo, uint maxRecordCount)
     {
@@ -50,6 +52,8 @@ public MappedMetricRepository(FileSystemInfo fileInfo, uint maxRecordCount)
             byte[] initContents = new byte[fileLength];
             initContents[VersionOffset] = SchemaVersion;
             fileStream.Write(initContents);
+            fileStream.Flush();
+            fileStream.Close();
             try
             {
                 File.Move(tmpFile.FullName, fileInfo.FullName);
@@ -62,14 +66,16 @@ public MappedMetricRepository(FileSystemInfo fileInfo, uint maxRecordCount)
         }
 
         MemoryMappedFile memoryMappedFile =
-            MemoryMappedFile.CreateFromFile(fileInfo.FullName, FileMode.OpenOrCreate, null, fileLength);
+            MemoryMappedFile.CreateFromFile(fileInfo.FullName, FileMode.Open, null, fileLength);
         _mappedByteBuffer = new MappedByteBuffer(memoryMappedFile);
         _unsafeBuffer = new UnsafeBuffer(_mappedByteBuffer);
         byte encodedSchemaVersion = _unsafeBuffer.GetByte(VersionOffset);
         if (encodedSchemaVersion != SchemaVersion)
         {
             throw new Exception($"Unexpected version: {encodedSchemaVersion}, expected: {SchemaVersion}");
         }
+
+        _repositoryIsDisposed = IsDisposed;
     }
 
     public IMetricCounter GetOrCreate(string identifier)
@@ -120,7 +126,7 @@ public IMetricCounter GetOrCreate(string identifier)
         _unsafeBuffer.PutByte(recordOffset + MetricTypeOffset, (byte)CounterType.Simple);
         return new MappedMetricCounter(
             new UnsafeBuffer(_unsafeBuffer.BufferPointer, recordOffset + LabelLength, MetricDataLength),
-            DateTimeEpochMillisSupplier.Instance);
+            DateTimeEpochMillisSupplier.Instance, _repositoryIsDisposed);
     }
 
     public void Read(IMetricValueReceiver metricValueReceiver)
@@ -149,10 +155,17 @@ public void Read(IMetricValueReceiver metricValueReceiver)
 
     public void Dispose()
     {
+        Volatile.Write(ref _isDisposed, true);
+        Thread.Sleep(1);
         _unsafeBuffer.Dispose();
         _mappedByteBuffer.Dispose();
     }
 
+    private bool IsDisposed()
+    {
+        return Volatile.Read(ref _isDisposed);
+    }
+
     private void ReleaseSpinLock()
     {
         _unsafeBuffer.PutIntVolatile(SpinLockOffset, ValueUnlocked);

diff --git a/DotMetrics/DotMetrics.csproj b/DotMetrics/DotMetrics.csproj
@@ -17,11 +17,12 @@
   </PropertyGroup>
 
   <ItemGroup>
+    <PackageReference Include="FourOTC.DotMetrics.Codec" />
     <PackageReference Include="InfluxDB.Collector" />
     <PackageReference Include="InfluxDB.LineProtocol" />
     <PackageReference Include="Microsoft.Diagnostics.NETCore.Client" />
     <PackageReference Include="Microsoft.Diagnostics.Tracing.TraceEvent" />
     <PackageReference Include="Microsoft.Extensions.Logging.Abstractions" />
   </ItemGroup>
 
-</Project>
+</Project>