A meaningful API surface is essential to the success of any library. To that end some methods are potentially unsuitable in SynchronizedValue. These need to have their original potential use cases examined to see if they're valid and significant enough to warrant their continued existence. If not they should be removed, along with unit tests, example code and documentation referencing them as part of a separate work item.

CHORE: Write comprehensive unit tests.

Request Summary

SemaphoreSlim, ReaderWriterLockSlim, and others have some well documented methods for implementing exclusive access to a resource. Identify and implement ways to simplify adherence to common mutual exclusion practices using these primitives.

In its simplest form synchronized access to a resource follows the pattern below:

SynchronizationPrimitive primitive= new();

// locking thread.
try {
     primitive.Wait();
     primitive.AcquireLock()
     // app code, do something.
}
finally 
{
   // this is sometimes multiple steps. Finally ensures this gets called even for exceptions.
   primitive.ReleaseLock();
}


// waiting thread.

try {
     primitive.Wait();
     primitive.AcquireLock()
     // app code, do something after lock is released.
}
finally 
{
   // this is sometimes multiple steps. Finally ensures this gets called even for exceptions.
   primitive.ReleaseLock();
}

However, it's remarkably easy to forgot the pattern, especially in light of the disparate API signatures among some of the dotnet synchronization primitives. For example ReaderWriterLockSlim has multiple types of locks, which can be upgraded to...etc, These need correct releasing based on what happened after the lock was acquired. While, SemaphoreSlim only has Wait and Release.

Resolution

Provide a set of extension methods that encapsulate and coordinate these calls, will help ensure these lock-release pairings are properly executed. Extension methods allow the point of use to write less code and improve readability.

Alternatives

Option 1: use an IDisposable to ensure release of the locks.

Choosing to implement this in an extension that returns a disposable would resemble:

public static class SynchronizationPrimitiveExtensions
{
   public IDisposable UseExclusively(this SynchronizationPrimitive primitive) => {
      primitive.Wait();

      return new SynchronizationPrimitiveLock(primitive);
   }
}

internal class SynchronizationPrimitiveLock : IDisposable
{
   private SynchronizationPrimitive primitive;

   internal SynchronizationPrimitiveLock(SynchronizationPrimitive primitive)
   {
      this.primitive = primitive;
      primitive.AcquireLock();
   }

   public void Dispose() { primitive.ReleaseLock(); }
}

Using the IDisposable instances changes the original code into:

SynchronizationPrimitive primitive= new();

// locking thread.
using(primitive.UseExclusively() 
{
     // app code, do something.
}

// waiting thread.
using(primitive.UseExclusively() 
{
     // app code, do something after lock is released.
}

Option 2: Use delegates in a wrapper method calling try..finally

public static class SynchronizationPrimitiveExtensions
{
   public void UseExclusively(this SynchronizationPrimitive primitive, Action action) {
      try
      {
         primitive.Wait();
         primitive.AcquireLock();
         action();
      }
      finally
      {
         primitive.ReleaseLock();
      }
   }
}

With the above pattern the point of use would look like:

SynchronizationPrimitive primitive= new();

// locking thread.
primitive.UseExclusively(()=> 
{
     // app code, do something.
});

// waiting thread.
primitive.UseExclusively(()=> 
{
     // app code, do something after lock is released.
});

delegate vs IDisposable Considerations

• IDisposable concerns:

  • Moderate concern: Failure to call Dispose when done with a resource may cause deadlocks and will cause locking for longer than intended. The risk of this happening is mitigated some with using var val = style of using syntax.
  • Minor concern: Done in a time consuming method using var val = primitive.UseExclusively(); can cause a resource to be locked longer than intended, but will get released.

• Delegate concerns really only exist for captured variables (i.e. a closure).

  • Moderate concern: Creating a closures takes a non-trivial amount of CPU time. Depending on the number of captured variables, this is slower than creating a trivial IDisposable which is important in performance-sensitive code executing in a tight loop[1].
  • Moderate concern: In a tight loop, executing closures also exert GC pressure proportional to the number of captured values and and number of scopes (more scopes = more generated capture classes; more captured variables at each scope = more fields in the generated capture classes). NOTE: ReSharper DPA routinely warns me about this when I'm analyzing code with closures. (According to this article they can be 50% slower than simple delegates)
  • Minor concern: Automated code formatters (especially those in a git hook) may indent the delegate's code in unexpected ways.

[1] - Performance sensitive code is code we want to ensure we're not imposing unnecessary CPU burdens on, but which is not performance-critical. Performance-critical code should always directly deal with the primitives.

Motivation

I've found myself trying this same pattern, over and over, when looking into selecting synchronization primitives. For non-performance critical code, simplifying improve code readability, and help ensure correct usage would be immensely useful.

User Story

As a library consumer, I want comprehensive example code, so I can understand how to appropriately use the library.

Acceptance Criteria

• All public classes have example usage in code to demonstrate how to use the class.
• All public and protected methods on classes have example usage code.
• All public classes have example usage documentation in the API documentation discussing the reasons for or for not using a class in particular ways.
• All public and protected methods on have example usage documentation in the API documentation discussing the reasons for or for not using a method in particular ways.

Work Ready Checklist

1. • Acceptance criteria defined
2. • Team understands acceptance criteria
3. • Team has defined solution / steps to satisfy acceptance criteria
4. • Acceptance criteria is verifiable / testable
5. • External / 3rd Party dependencies identified (none)