Merge pull request #1393 from mvphelps/ToTensorMemoryLeakFix

NiklasGustafsson · web-flow · commit 985648707317 · 2024-10-23T14:10:06.000-07:00
Fix memory leaks in .ToTensor, resolves #1392
diff --git a/RELEASENOTES.md b/RELEASENOTES.md
@@ -12,7 +12,8 @@ __Bug Fixes__:
 #1383 `torch.linalg.vector_norm`: Make `ord`-argument optional, as specified in docs<br/>
 #1385 PackedSequence now participates in the DisposeScope system at the same level as Tensor objects.<br/>
 #1387 Attaching tensor to a DisposeScope no longer makes Statistics.DetachedFromScopeCount go negative.<br/>
-#1390 DisposeScopeManager.Statistics now includes DisposedOutsideScopeCount and AttachedToScopeCount. ThreadTotalLiveCount is now exact instead of approximate. ToString gives a useful debug string, and documentation is added for how to troubleshoot memory leaks. Also DisposeScopeManager.Statistics.TensorStatistics and DisposeScopeManager.Statistics.PackedSequenceStatistics provide separate metrics for these objects.
+#1390 DisposeScopeManager.Statistics now includes DisposedOutsideScopeCount and AttachedToScopeCount. ThreadTotalLiveCount is now exact instead of approximate. ToString gives a useful debug string, and documentation is added for how to troubleshoot memory leaks. Also DisposeScopeManager.Statistics.TensorStatistics and DisposeScopeManager.Statistics.PackedSequenceStatistics provide separate metrics for these objects.<br/>
+#1392 ToTensor() extension method memory leaks fixed.<br/>
 
 # NuGet Version 0.103.0
 
diff --git a/src/TorchSharp/Tensor/Factories/tensor_Complex.cs b/src/TorchSharp/Tensor/Factories/tensor_Complex.cs
@@ -18,9 +18,7 @@ public static Tensor tensor(System.Numerics.Complex scalar, ScalarType? dtype =
             device = InitializeDevice(device);
             var handle = THSTensor_newComplexFloat64Scalar(scalar.Real, scalar.Imaginary, (int)device.type, device.index, requires_grad);
             if (handle == IntPtr.Zero) { CheckForErrors(); }
-            var tensor = new Tensor(handle);
-            tensor = dtype.HasValue ? tensor.to(dtype.Value, device) : tensor.to(device);
-            return tensor;
+            return InstantiateTensorWithLeakSafeTypeChange(handle, dtype);
         }
 
         /// <summary>
diff --git a/src/TorchSharp/Tensor/Factories/tensor_double.cs b/src/TorchSharp/Tensor/Factories/tensor_double.cs
@@ -18,43 +18,27 @@ public static Tensor tensor(double scalar, ScalarType? dtype = null, Device? dev
             device = InitializeDevice(device);
             var handle = THSTensor_newFloat64Scalar(scalar, (int)device.type, device.index, requires_grad);
             if (handle == IntPtr.Zero) { CheckForErrors(); }
-            var tensor = new Tensor(handle);
-            tensor = dtype.HasValue ? tensor.to(dtype.Value, device) : tensor.to(device);
-            return tensor;
+            return InstantiateTensorWithLeakSafeTypeChange(handle, dtype);
         }
 
         /// <summary>
-        /// Create a scalar tensor from a single value
+        /// Create a scalar complex number tensor from a tuple of (real, imaginary)
         /// </summary>
         public static Tensor tensor((double Real, double Imaginary) scalar, ScalarType? dtype = null, Device? device = null, bool requires_grad = false)
         {
-            device = InitializeDevice(device);
-            var handle = THSTensor_newComplexFloat64Scalar(scalar.Real, scalar.Imaginary, (int)device.type, device.index, requires_grad);
-            if (handle == IntPtr.Zero) { CheckForErrors(); }
-            var tensor = new Tensor(handle);
-            if (device is { }) {
-                tensor = dtype.HasValue ? tensor.to(dtype.Value, device) : tensor.to(device);
-            } else if (dtype.HasValue) {
-                tensor = tensor.to_type(dtype.Value);
-            }
-            return tensor;
+            return tensor(scalar.Real, scalar.Imaginary, dtype, device, requires_grad);
         }
 
         /// <summary>
-        /// Create a scalar tensor from a single value
+        /// Create a scalar complex number tensor from independent real and imaginary components
         /// </summary>
         public static Tensor tensor(double real, double imaginary, ScalarType? dtype = null, Device? device = null, bool requires_grad = false)
         {
+
             device = InitializeDevice(device);
             var handle = THSTensor_newComplexFloat64Scalar(real, imaginary, (int)device.type, device.index, requires_grad);
             if (handle == IntPtr.Zero) { CheckForErrors(); }
-            var tensor = new Tensor(handle);
-            if (device is { }) {
-                tensor = dtype.HasValue ? tensor.to(dtype.Value, device) : tensor.to(device);
-            } else if (dtype.HasValue) {
-                tensor = tensor.to_type(dtype.Value);
-            }
-            return tensor;
+            return InstantiateTensorWithLeakSafeTypeChange(handle, dtype);
         }
 
         /// <summary>
diff --git a/src/TorchSharp/Tensor/Factories/tensor_float.cs b/src/TorchSharp/Tensor/Factories/tensor_float.cs
@@ -22,37 +22,22 @@ public static Tensor tensor(float scalar, Device? device = null, bool requires_g
         }
 
         /// <summary>
-        /// Create a scalar tensor from a single value
+        /// Create a scalar complex number tensor from independent real and imaginary components
         /// </summary>
         public static Tensor tensor(float real, float imaginary, ScalarType? dtype = null, Device? device = null, bool requires_grad = false)
         {
             device = InitializeDevice(device);
             var handle = THSTensor_newComplexFloat32Scalar(real, imaginary, (int)device.type, device.index, requires_grad);
             if (handle == IntPtr.Zero) { CheckForErrors(); }
-            var tensor = new Tensor(handle);
-            if (device is { }) {
-                tensor = dtype.HasValue ? tensor.to(dtype.Value, device) : tensor.to(device);
-            } else if (dtype.HasValue) {
-                tensor = tensor.to_type(dtype.Value);
-            }
-            return tensor;
+            return InstantiateTensorWithLeakSafeTypeChange(handle, dtype);
         }
 
         /// <summary>
-        /// Create a scalar tensor from a single value
+        /// Create a scalar complex number tensor from a tuple of (real, imaginary)
         /// </summary>
         public static Tensor tensor((float Real, float Imaginary) scalar, ScalarType? dtype = null, Device? device = null, bool requires_grad = false)
         {
-            device = InitializeDevice(device);
-            var handle = THSTensor_newComplexFloat32Scalar(scalar.Real, scalar.Imaginary, (int)device.type, device.index, requires_grad);
-            if (handle == IntPtr.Zero) { CheckForErrors(); }
-            var tensor = new Tensor(handle);
-            if (device is { }) {
-                tensor = dtype.HasValue ? tensor.to(dtype.Value, device) : tensor.to(device);
-            } else if (dtype.HasValue) {
-                tensor = tensor.to_type(dtype.Value);
-            }
-            return tensor;
+            return tensor(scalar.Real, scalar.Imaginary, dtype: dtype, device: device);
         }
 
         /// <summary>
diff --git a/src/TorchSharp/Tensor/Factories/tensor_long.cs b/src/TorchSharp/Tensor/Factories/tensor_long.cs
@@ -18,13 +18,7 @@ public static Tensor tensor(long scalar, ScalarType? dtype = null, Device? devic
             device = InitializeDevice(device);
             var handle = THSTensor_newInt64Scalar(scalar, (int)device.type, device.index, requires_grad);
             if (handle == IntPtr.Zero) { CheckForErrors(); }
-            var tensor = new Tensor(handle);
-            if (device is { }) {
-                tensor = dtype.HasValue ? tensor.to(dtype.Value, device) : tensor.to(device);
-            } else if (dtype.HasValue) {
-                tensor = tensor.to_type(dtype.Value);
-            }
-            return tensor;
+            return InstantiateTensorWithLeakSafeTypeChange(handle, dtype);
         }
 
         /// <summary>
diff --git a/src/TorchSharp/Tensor/Tensor.cs b/src/TorchSharp/Tensor/Tensor.cs
@@ -7402,5 +7402,15 @@ public static Tensor WrappedTensorDisposeScope(Func<Tensor> expr)
             var result = expr();
             return result.MoveToOuterDisposeScope();
         }
+        internal static Tensor InstantiateTensorWithLeakSafeTypeChange(IntPtr handle, ScalarType? dtype)
+        {
+            var tensor = new Tensor(handle);
+            if (dtype.HasValue && tensor.dtype != dtype.Value) {
+                var typed = tensor.to_type(dtype.Value);
+                tensor.Dispose();
+                return typed;
+            }
+            return tensor;
+        }
     }
 }
diff --git a/test/TorchSharpTest/TestDisposeScopesStatisticsTensor.cs b/test/TorchSharpTest/TestDisposeScopesStatisticsTensor.cs
@@ -73,26 +73,5 @@ public void DisposingScopeAfterDetachingDoesNothing()
             AssertTensorCounts(0, 0, 1, 0, 0, 1, 1);
             AssertTotalsCounts(0, 0, 1, 0, 0, 1, 1);
         }
-
-        [Fact]
-        public void ToTensorCreatesOrphanedTensorButDisposeScopeCleansItUp()
-        {
-            //Defect: This needs fixing but is unrelated to the commit that discovered
-            //it - adding better lifetime statistics. ToTensor() leaks 1 tensor
-            //every time it is called.
-            var scope = torch.NewDisposeScope();
-            var stats = DisposeScopeManager.Statistics;
-            stats.Reset();
-            var a1 = 1.ToTensor();
-            Assert.Equal(2, stats.CreatedInScopeCount);
-            //Should be 1, or can remain 0 if CreatedInScope becomes 1.
-            Assert.Equal(0, stats.DisposedInScopeCount);
-            a1.Dispose();
-            Assert.Equal(1, stats.DisposedInScopeCount);
-
-            //Should not need this if no orphan.
-            scope.Dispose();
-            Assert.Equal(2, stats.DisposedInScopeCount);
-        }
     }
 }
diff --git a/test/TorchSharpTest/TestDisposeScopesStatisticsTensorUnscoped.cs b/test/TorchSharpTest/TestDisposeScopesStatisticsTensorUnscoped.cs
@@ -114,23 +114,5 @@ public void AttachAgainIsSameAsMoveToOtherAndStatsDoNotChange()
             AssertTensorCounts(1, 0, 0, 0, 1, 0, 1);
             AssertTotalsCounts(1, 0, 0, 0, 1, 0, 1);
         }
-
-        [Fact]
-        public void ToTensorCreatesOrphanedTensor()
-        {
-            //Defect: This needs fixing but is unrelated to the commit that discovered
-            //it - adding better lifetime statistics. ToTensor() leaks 1 tensor
-            // //every time it is called.
-            var stats = DisposeScopeManager.Statistics;
-            stats.Reset();
-            var a1 = 1.ToTensor();
-            //Should be 1 ideally. If it remains two...
-            Assert.Equal(2, stats.CreatedOutsideScopeCount);
-            // ... this should be 1
-            Assert.Equal(0, stats.DisposedOutsideScopeCount);
-            a1.Dispose();
-            //... and this should also be 2
-            Assert.Equal(1, stats.DisposedOutsideScopeCount);
-        }
     }
 }
diff --git a/test/TorchSharpTest/TestTorchTensor.cs b/test/TorchSharpTest/TestTorchTensor.cs
@@ -4,6 +4,7 @@
 using System.Globalization;
 using System.IO;
 using System.Linq;
+using System.Numerics;
 using System.Runtime.InteropServices;
 using Xunit;
 using Xunit.Sdk;
@@ -3228,6 +3229,66 @@ public void ScalarToTensor3()
                 Assert.Equal(-1.0, (double)tensor);
             }
         }
+        [Fact]
+        [TestOf(nameof(Tensor))]
+        public void ScalarToTensorDoesNotLeakMemory()
+        {
+            AssertTensorDoesNotLeak(()=>{
+                Tensor tensor = 1;
+                return tensor;
+            });
+            AssertTensorDoesNotLeak(() => ((byte)1).ToTensor());
+            AssertTensorDoesNotLeak(() => ((sbyte)-1).ToTensor());
+            AssertTensorDoesNotLeak(() => ((short)-1).ToTensor());
+            AssertTensorDoesNotLeak(() => ((long)-1).ToTensor());
+            AssertTensorDoesNotLeak(() => ((float)-1).ToTensor());
+            AssertTensorDoesNotLeak(() => ((double)-1).ToTensor());
+        }
+
+        [Fact]
+        [TestOf(nameof(Tensor))]
+        public void ScalarArrayToTensorDoesNotLeakMemory()
+        {
+            AssertTensorDoesNotLeak(() => (new byte[]{1}).ToTensor(new long[]{1}));
+            AssertTensorDoesNotLeak(() => (new sbyte[]{-1}).ToTensor(new long[]{1}));
+            AssertTensorDoesNotLeak(() => (new short[]{-1}).ToTensor(new long[]{1}));
+            AssertTensorDoesNotLeak(() => (new long[]{-1}).ToTensor(new long[]{1}));
+            AssertTensorDoesNotLeak(() => (new float[]{-1}).ToTensor(new long[]{1}));
+            AssertTensorDoesNotLeak(() => (new double[]{-1}).ToTensor(new long[]{1}));
+        }
+
+        [Fact]
+        [TestOf(nameof(Tensor))]
+        public void ComplexNumberOfDoubleDoesNotLeakMemory()
+        {
+            AssertTensorDoesNotLeak(() => ( torch.tensor((double)-1, (double)-2)));
+            AssertTensorDoesNotLeak(() => ( torch.tensor(((double)-1, (double)-2))));
+        }
+
+        [Fact]
+        [TestOf(nameof(Tensor))]
+        public void ComplexNumberOfFloatDoesNotLeakMemory()
+        {
+            AssertTensorDoesNotLeak(() => (torch.tensor((float)-1, (float)-2)));
+            AssertTensorDoesNotLeak(() => (torch.tensor(((float)-1, (float)-2))));
+        }
+
+        [Fact]
+        [TestOf(nameof(Tensor))]
+        public void DotNetComplexNumberDoesNotLeakMemory()
+        {
+            AssertTensorDoesNotLeak(() => (torch.tensor(new Complex(1, 2))));
+        }
+
+        private void AssertTensorDoesNotLeak(Func<Tensor> createTensorFunc)
+        {
+            var stats = DisposeScopeManager.Statistics.TensorStatistics;
+            stats.Reset();
+            using (Tensor tensor = createTensorFunc()) {
+                Assert.Equal(1, stats.ThreadTotalLiveCount);
+            }
+            Assert.Equal(0, stats.ThreadTotalLiveCount);
+        }
 
         [Fact]
         [TestOf(nameof(Tensor))]

Original file line number	Diff line number	Diff line change
`@@ -7402,5 +7402,15 @@ public static Tensor WrappedTensorDisposeScope(Func<Tensor> expr)`
`7402`	`7402`	`var result = expr();`
`7403`	`7403`	`return result.MoveToOuterDisposeScope();`
`7404`	`7404`	`}`
	`7405`	`+ internal static Tensor InstantiateTensorWithLeakSafeTypeChange(IntPtr handle, ScalarType? dtype)`
	`7406`	`+ {`
	`7407`	`+ var tensor = new Tensor(handle);`
	`7408`	`+ if (dtype.HasValue && tensor.dtype != dtype.Value) {`
	`7409`	`+ var typed = tensor.to_type(dtype.Value);`
	`7410`	`+ tensor.Dispose();`
	`7411`	`+ return typed;`
	`7412`	`+ }`
	`7413`	`+ return tensor;`
	`7414`	`+ }`
`7405`	`7415`	`}`
`7406`	`7416`	`}`