diff --git a/src/SimpleInjector/Internals/CyclicDependencyValidator.cs b/src/SimpleInjector/Internals/CyclicDependencyValidator.cs
index 92d047e2..dc961cf2 100644
--- a/src/SimpleInjector/Internals/CyclicDependencyValidator.cs
+++ b/src/SimpleInjector/Internals/CyclicDependencyValidator.cs
@@ -3,18 +3,32 @@
namespace SimpleInjector.Internals
{
- using System.Threading;
+ using System;
+ using System.Collections.Generic;
///
/// Allows verifying whether a given type has a direct or indirect dependency on itself. Verifying is done
/// by preventing recursive calls to an InstanceProducer. A CyclicDependencyValidator instance checks a
/// single InstanceProducer and therefore a single service type.
+ /// DESIGN: An instance or expression can be requested from multiple threads simultaniously, which means
+ /// that this class must be thread-safe but also prevent reporting false positives in cyclic dependencies
+ /// when called from multiple thread. Originally, this was done by wrapping a ThreadLocal{bool}, but this
+ /// lead to memory leaks, as in most cases the ThreadLocal{bool} couldn't be disposed. THat's why this
+ /// class now wraps a list of currently entered threads. Because the list is nulled when there are no
+ /// entering threads, this is a very memory-efficient solution.
///
internal sealed class CyclicDependencyValidator
{
- private readonly ThreadLocal producerVisited = new ThreadLocal();
private readonly InstanceProducer producer;
+ // If needed, we can do an extra optimization, which is to have an extra int field for the first
+ // entered thread. This prevents the list from having to be newed in most cases, as typically there
+ // is only on thread entering at the same time. This does complicate the code of this class though.
+ // Typically, however, we're not that concerned with producing garbage during the warmup phase.
+ // There are quite a few cases were we produce extensive garbage. Producing garbage in the happy
+ // path, however, is somethings we prevent at all costs.
+ private List? enteredThreads;
+
internal CyclicDependencyValidator(InstanceProducer producer)
{
this.producer = producer;
@@ -23,16 +37,70 @@ internal CyclicDependencyValidator(InstanceProducer producer)
// Checks whether this is a recursive call (and thus a cyclic dependency) and throw in that case.
internal void Check()
{
- if (this.producerVisited.Value)
+ lock (this)
+ {
+ if (this.IsCurrentThreadReentering())
+ {
+ throw new CyclicDependencyException(
+ this.producer, this.producer.Registration.ImplementationType);
+ }
+
+ this.MarkCurrentThreadAsEntering();
+ }
+ }
+
+ // Resets the validator to its initial state.
+ internal void Reset()
+ {
+ lock (this)
+ {
+ this.MarkCurrentThreadAsLeaving();
+ }
+ }
+
+ private bool IsCurrentThreadReentering()
+ {
+ if (this.enteredThreads != null)
+ {
+ int currentThreadId = Environment.CurrentManagedThreadId;
+
+ foreach (int threadId in this.enteredThreads)
+ {
+ if (threadId == currentThreadId) return true;
+ }
+ }
+
+ return false;
+ }
+
+ private void MarkCurrentThreadAsEntering()
+ {
+ int currentThreadId = Environment.CurrentManagedThreadId;
+
+ if (this.enteredThreads is null)
{
- throw new CyclicDependencyException(
- this.producer, this.producer.Registration.ImplementationType);
+ this.enteredThreads = new(1);
}
- this.producerVisited.Value = true;
+ this.enteredThreads.Add(currentThreadId);
}
- // Resets the validator to its initial state.
- internal void Reset() => this.producerVisited.Value = false;
+ private void MarkCurrentThreadAsLeaving()
+ {
+ if (this.enteredThreads != null)
+ {
+ this.enteredThreads.Remove(Environment.CurrentManagedThreadId);
+
+ if (this.enteredThreads.Count == 0)
+ {
+ // Nullify the list. Although this causes the production of way more
+ // garbage during the warm-up phase when the instance producer is
+ // depended upon a lot, it ensures that the least amount of memory
+ // is used when the warmup phase is complete and all expression trees
+ // have been compiled.
+ this.enteredThreads = null;
+ }
+ }
+ }
}
}
\ No newline at end of file