Problem
I have written a type with the following public API:
public sealed class InitializationGuard
{
public bool IsUninitialized
{
get;
}
public bool IsInitializing
{
get;
}
public bool IsInitialized
{
get;
}
public InitializationTransaction BeginInitialization();
public bool TryBeginInitialization(out InitializationTransaction initializationTransaction);
public void EnsureInitialized();
}
The intention is to use it in services that require initialization, possibly asynchronous with multiple threads attempting to initialize them at once. Something like this:
public sealed class SomeService : ISomeService
{
private readonly InitializationGuard initializationGuard = new InitializationGuard();
public async Task Initialize()
{
using (var transaction = this.initializationGuard.BeginInitialization())
{
await SomeInitLogic();
transaction.Commit();
}
}
public async Task DoSomethingThatRequiresInitialization()
{
this.initializationGuard.EnsureInitialized();
await SomeOtherLogic();
}
}
The idea is to save having to write this kind of guard logic in every service.
My questions are:
- is this a valid approach?
- am I kidding myself that this is truly thread-safe?
- is there perhaps a better approach to the implementation? I’m not particularly happy with the pattern that emerges when using
TryBeginInitialization()
, but am not really sure of any viable alternative
Here is the full code (apart from DisposableBase
, which just provides thread-safety when disposing):
InitializationGuard
using System.Diagnostics;
using System.Threading;
[DebuggerDisplay("Initialized: {IsInitialized}")]
public sealed class InitializationGuard
{
private const int Uninitialized = 0;
private const int Initializing = 1;
private const int Initialized = 2;
private int initializationState;
public bool IsUninitialized
{
get { return this.IsInInitializationState(Uninitialized); }
}
public bool IsInitializing
{
get { return this.IsInInitializationState(Initializing); }
}
public bool IsInitialized
{
get { return this.IsInInitializationState(Initialized); }
}
public InitializationTransaction BeginInitialization()
{
InitializationTransaction initializationTransaction;
if (!this.TryBeginInitialization(out initializationTransaction))
{
throw new InitializationException("Initialization already instigated.");
}
return initializationTransaction;
}
public bool TryBeginInitialization(out InitializationTransaction initializationTransaction)
{
if (Interlocked.CompareExchange(ref this.initializationState, Initializing, Uninitialized) != Uninitialized)
{
initializationTransaction = null;
return false;
}
initializationTransaction = new InitializationTransaction(this);
return true;
}
public void EnsureInitialized()
{
if (!this.IsInitialized)
{
throw new InitializationException("Not yet initialized.");
}
}
internal void InitializationTransactionComplete(InitializationTransaction initializationTransaction)
{
var newState = initializationTransaction.IsCommitted ? Initialized : Uninitialized;
Interlocked.Exchange(ref this.initializationState, newState);
}
private bool IsInInitializationState(int initializationState)
{
return Interlocked.CompareExchange(ref this.initializationState, initializationState, initializationState) == initializationState;
}
}
InitializationTransaction
public sealed class InitializationTransaction : DisposableBase
{
private readonly InitializationGuard owner;
private volatile bool isCommitted;
internal InitializationTransaction(InitializationGuard owner)
{
this.owner = owner;
}
public bool IsCommitted
{
get { return this.isCommitted; }
}
public void Commit()
{
this.isCommitted = true;
}
protected override void Dispose(bool disposing)
{
base.Dispose(disposing);
if (disposing)
{
this.owner.InitializationTransactionComplete(this);
}
}
}
InitializationException
using System;
public sealed class InitializationException : Exception
{
public InitializationException()
{
}
public InitializationException(string message)
: base(message)
{
}
public InitializationException(string message, Exception innerException)
: base(message, innerException)
{
}
}
InitializationGuardFixture
using Xunit;
public sealed class InitializationGuardFixture
{
[Fact]
public void an_in_flight_initialization_has_a_state_of_initializing()
{
var initializationGuard = new InitializationGuard();
using (initializationGuard.BeginInitialization())
{
Assert.True(initializationGuard.IsInitializing);
}
}
[Fact]
public void an_uncommitted_initialization_reverts_the_state_to_uninitialized()
{
var initializationGuard = new InitializationGuard();
initializationGuard
.BeginInitialization()
.Dispose();
Assert.True(initializationGuard.IsUninitialized);
}
[Fact]
public void a_committed_initialization_changes_the_state_to_initialized()
{
var initializationGuard = new InitializationGuard();
using (var initializationTransaction = initializationGuard.BeginInitialization())
{
initializationTransaction.Commit();
}
Assert.True(initializationGuard.IsInitialized);
}
[Fact]
public void cannot_begin_initialization_if_already_initializing()
{
var initializationGuard = new InitializationGuard();
using (initializationGuard.BeginInitialization())
{
var ex = Assert.Throws<InitializationException>(() => initializationGuard.BeginInitialization());
Assert.Equal("Initialization already instigated.", ex.Message);
}
}
[Fact]
public void cannot_begin_initialization_if_already_initialized()
{
var initializationGuard = new InitializationGuard();
using (var initializationTransaction = initializationGuard.BeginInitialization())
{
initializationTransaction.Commit();
}
var ex = Assert.Throws<InitializationException>(() => initializationGuard.BeginInitialization());
Assert.Equal("Initialization already instigated.", ex.Message);
}
[Fact]
public void trying_to_initialize_returns_true_if_initialization_not_yet_instigated()
{
var initializationGuard = new InitializationGuard();
InitializationTransaction initializationTransaction;
Assert.True(initializationGuard.TryBeginInitialization(out initializationTransaction));
Assert.NotNull(initializationTransaction);
}
[Fact]
public void trying_to_initialize_returns_false_if_already_initializing()
{
var initializationGuard = new InitializationGuard();
using (initializationGuard.BeginInitialization())
{
InitializationTransaction initializationTransaction;
Assert.False(initializationGuard.TryBeginInitialization(out initializationTransaction));
}
}
[Fact]
public void trying_to_initialize_returns_false_if_already_initialized()
{
var initializationGuard = new InitializationGuard();
InitializationTransaction initializationTransaction;
using (initializationTransaction = initializationGuard.BeginInitialization())
{
initializationTransaction.Commit();
}
Assert.False(initializationGuard.TryBeginInitialization(out initializationTransaction));
}
[Fact]
public void ensure_initialized_succeeds_if_initialized()
{
var initializationGuard = new InitializationGuard();
using (var initializationTransaction = initializationGuard.BeginInitialization())
{
initializationTransaction.Commit();
}
initializationGuard.EnsureInitialized();
}
[Fact]
public void ensure_initialized_fails_if_not_yet_initialized()
{
var initializationGuard = new InitializationGuard();
var ex = Assert.Throws<InitializationException>(() => initializationGuard.EnsureInitialized());
Assert.Equal("Not yet initialized.", ex.Message);
}
}
Solution
private const int Uninitialized = 0;
private const int Initializing = 1;
private const int Initialized = 2;
These constants really smell like they want to be an enum
.
public bool IsUninitialized
{
get { return this.IsInInitializationState(Uninitialized); }
}
public bool IsInitializing
{
get { return this.IsInInitializationState(Initializing); }
}
public bool IsInitialized
{
get { return this.IsInInitializationState(Initialized); }
}
With an enum InitializationState
you wouldn’t need these three getters, and could simply make IsInInitializationState
public, taking an InitializationState
parameter instead of an int
, and this.initializationState
would be an InitializationState
instead of an int
, too. The naming already seems to be asking for that.