From fb2cf6509d9c9b6987acba8b5b53b1f1b7998314 Mon Sep 17 00:00:00 2001 From: "Ilya.Usov" Date: Wed, 15 Jan 2025 14:16:12 +0100 Subject: [PATCH 1/3] Request diagnostic data and only then set logError flag --- rd-net/Lifetimes/Lifetimes/LifetimeDefinition.cs | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/rd-net/Lifetimes/Lifetimes/LifetimeDefinition.cs b/rd-net/Lifetimes/Lifetimes/LifetimeDefinition.cs index b2bb78f32..cae5bd47b 100644 --- a/rd-net/Lifetimes/Lifetimes/LifetimeDefinition.cs +++ b/rd-net/Lifetimes/Lifetimes/LifetimeDefinition.cs @@ -460,16 +460,21 @@ public void Terminate() + "This may happen either because of the ExecuteIfAlive failed to complete in a timely manner. In the case there will be following error messages." + Environment.NewLine + "This is also possible if the thread waiting for the termination wasn't able to receive execution time during the wait in SpinWait.spinUntil, so it has missed the fact that the lifetime was terminated in time."); - ourLogErrorAfterExecution.InterlockedUpdate(ref myState, true); #if !NET35 if (AdditionalDiagnostics is { } additionalDiagnostics) { - Log.Catch(() => + try { ourAdditionalDiagnosticsStorage.AddData(this, additionalDiagnostics.GetAdditionalDiagnosticsIfExecutionWasNotCancelledByTimeoutAsync(Lifetime)); - }); + } + catch (Exception e) + { + Log.Error(e); + } } #endif + + ourLogErrorAfterExecution.InterlockedUpdate(ref myState, true); } if (!IncrementStatusIfEqualsTo(LifetimeStatus.Canceling)) From ebece1777824328fac4123af052fa1c7dd4183f2 Mon Sep 17 00:00:00 2001 From: "Ilya.Usov" Date: Wed, 15 Jan 2025 14:47:59 +0100 Subject: [PATCH 2/3] Log error on accessing non-initialized lifetime to avoid memory leaks. Add `LogErrorIfLifetimeIsNotInitialized` to be able to disable this behaviour --- rd-net/Lifetimes/Lifetimes/Lifetime.cs | 24 ++++++++++++++++++++++-- 1 file changed, 22 insertions(+), 2 deletions(-) diff --git a/rd-net/Lifetimes/Lifetimes/Lifetime.cs b/rd-net/Lifetimes/Lifetimes/Lifetime.cs index 187c0c002..5c01e6a3e 100644 --- a/rd-net/Lifetimes/Lifetimes/Lifetime.cs +++ b/rd-net/Lifetimes/Lifetimes/Lifetime.cs @@ -110,14 +110,34 @@ public enum LifetimeTerminationTimeoutKind { private readonly LifetimeDefinition? myDefinition; - internal LifetimeDefinition Definition => myDefinition ?? LifetimeDefinition.Eternal; - + internal LifetimeDefinition Definition + { + get + { + var def = myDefinition; + if (def != null) return def; + + if (LogErrorIfLifetimeIsNotInitialized) + { + Log.Root.Error("Lifetime is not initialized. " + + "This may cause a memory leak as default(Lifetime) is treated as Eternal. " + + "Please provide a properly initialized Lifetime or use `Lifetime?` if you need to handle both cases. " + + "Use Lifetime.Eternal explicitly if that behavior is intended."); + } + + return LifetimeDefinition.Eternal; + } + } + //ctor internal Lifetime(LifetimeDefinition definition) { myDefinition = definition; } + [PublicAPI] + public static bool LogErrorIfLifetimeIsNotInitialized = true; + #if !NET35 /// From 87fa5f69ea0a3fb7c2be7410bcd932b31e0d761b Mon Sep 17 00:00:00 2001 From: "Ilya.Usov" Date: Wed, 15 Jan 2025 15:29:06 +0100 Subject: [PATCH 3/3] Add IsInitialized + IsUninitialized api Fix tests Check IsInitialized in ValueLifetimed + add an assertion that the passed lifetime is initialized --- rd-net/Lifetimes/Lifetimes/Lifetime.cs | 35 ++++++++++++++----- rd-net/Lifetimes/Lifetimes/ValueLifetimed.cs | 4 ++- .../Test.Lifetimes/Lifetimes/LifetimeTest.cs | 25 ++++++++----- 3 files changed, 47 insertions(+), 17 deletions(-) diff --git a/rd-net/Lifetimes/Lifetimes/Lifetime.cs b/rd-net/Lifetimes/Lifetimes/Lifetime.cs index 5c01e6a3e..f7d2c272f 100644 --- a/rd-net/Lifetimes/Lifetimes/Lifetime.cs +++ b/rd-net/Lifetimes/Lifetimes/Lifetime.cs @@ -117,18 +117,26 @@ internal LifetimeDefinition Definition var def = myDefinition; if (def != null) return def; - if (LogErrorIfLifetimeIsNotInitialized) - { - Log.Root.Error("Lifetime is not initialized. " + - "This may cause a memory leak as default(Lifetime) is treated as Eternal. " + - "Please provide a properly initialized Lifetime or use `Lifetime?` if you need to handle both cases. " + - "Use Lifetime.Eternal explicitly if that behavior is intended."); - } - + Assertion.Assert(IsUninitialized); + + AssertInitialized(); return LifetimeDefinition.Eternal; } } + internal void AssertInitialized() + { + if (!Mode.IsAssertion) return; + + if (LogErrorIfLifetimeIsNotInitialized && IsUninitialized) + { + Log.Root.Error("Lifetime is not initialized. " + + "This may cause a memory leak as default(Lifetime) is treated as Eternal. " + + "Please provide a properly initialized Lifetime or use `Lifetime?` if you need to handle both cases. " + + "Use Lifetime.Eternal explicitly if that behavior is intended."); + } + } + //ctor internal Lifetime(LifetimeDefinition definition) { @@ -167,6 +175,17 @@ internal Lifetime(LifetimeDefinition definition) /// Whether current lifetime is equal to and never be terminated /// [PublicAPI] public bool IsEternal => Definition.IsEternal; + + + /// + /// Whether current lifetime is not properly initialized (created by default(Lifetime)) + /// + [PublicAPI] public bool IsUninitialized => !IsInitialized; + + /// + /// Whether current lifetime is properly initialized + /// + [PublicAPI] public bool IsInitialized => myDefinition != null; /// /// Is of this lifetime equal to diff --git a/rd-net/Lifetimes/Lifetimes/ValueLifetimed.cs b/rd-net/Lifetimes/Lifetimes/ValueLifetimed.cs index 90a0bc214..e6c70822b 100644 --- a/rd-net/Lifetimes/Lifetimes/ValueLifetimed.cs +++ b/rd-net/Lifetimes/Lifetimes/ValueLifetimed.cs @@ -20,13 +20,15 @@ public void Deconstruct(out Lifetime lifetime, out T? value) public ValueLifetimed(Lifetime lifetime, T value) { + lifetime.AssertInitialized(); + Lifetime = lifetime; Value = value; } public void ClearValueIfNotAlive() { - if (!Lifetime.IsAlive) + if (Lifetime is { IsInitialized: true, IsAlive: false }) Value = default(T); } } diff --git a/rd-net/Test.Lifetimes/Lifetimes/LifetimeTest.cs b/rd-net/Test.Lifetimes/Lifetimes/LifetimeTest.cs index 9455c3383..eae8a71d7 100644 --- a/rd-net/Test.Lifetimes/Lifetimes/LifetimeTest.cs +++ b/rd-net/Test.Lifetimes/Lifetimes/LifetimeTest.cs @@ -152,16 +152,25 @@ public void TestEternal() [Test] public void TestEquals() { - Lifetime eternal = default; - Assert.AreEqual(Lifetime.Eternal, eternal); - Assert.AreEqual(Lifetime.Eternal, Lifetime.Eternal); - Assert.AreEqual(eternal, eternal); + var old = Lifetime.LogErrorIfLifetimeIsNotInitialized; + Lifetime.LogErrorIfLifetimeIsNotInitialized = false; + try + { + Lifetime eternal = default; + Assert.AreEqual(Lifetime.Eternal, eternal); + Assert.AreEqual(Lifetime.Eternal, Lifetime.Eternal); + Assert.AreEqual(eternal, eternal); - Assert.True(Lifetime.Eternal == eternal); + Assert.True(Lifetime.Eternal == eternal); - Assert.AreNotEqual(Lifetime.Eternal, Lifetime.Terminated); - Assert.False(Lifetime.Eternal == Lifetime.Terminated); - Assert.False(eternal == Lifetime.Terminated); + Assert.AreNotEqual(Lifetime.Eternal, Lifetime.Terminated); + Assert.False(Lifetime.Eternal == Lifetime.Terminated); + Assert.False(eternal == Lifetime.Terminated); + } + finally + { + Lifetime.LogErrorIfLifetimeIsNotInitialized = old; + } } [Test]