7

According to this question it should be guaranteed that static fields that i use are initialized:

10.4.5.1 Static field initialization:

The static field variable initializers of a class correspond to a sequence of assignments that are executed in the textual order in which they appear in the class declaration. If a static constructor (Section 10.11) exists in the class, execution of the static field initializers occurs immediately prior to executing that static constructor. Otherwise, the static field initializers are executed at an implementation-dependent time prior to the first use of a static field of that class.

I've encountered a strange case where this doesn't seem to be true. I have two classes which have a circular dependency on each other and where a NullReferenceException is thrown.

I was able to reproduce this issue in following simplified sample, have a look:

public class SessionManager
{
    //// static constructor doesn't matter
    //static SessionManager()
    //{
    //    _instance = new SessionManager();
    //}

    private static SessionManager _instance = new SessionManager();
    public static SessionManager GetInstance()
    {
        return _instance;
    }

    public SessionManager()
    {
        Console.WriteLine($"{nameof(SessionManager)} constructor called");
        this.RecoverState();
    }

    public bool RecoverState()
    {
        Console.WriteLine($"{nameof(RecoverState)} called");
        List<SessionInfo> activeSessionsInDb = SessionManagerDatabase.GetInstance().LoadActiveSessionsFromDb();
        // ...
        return true;
    }

    public List<SessionInfo> GetAllActiveSessions()
    {
        Console.WriteLine($"{nameof(GetAllActiveSessions)} called");
        return new List<SessionInfo>();
    }
}

public class SessionManagerDatabase
{
    //// static constructor doesn't matter
    //static SessionManagerDatabase()
    //{
    //    _instance = new SessionManagerDatabase();
    //}

    private static readonly SessionManagerDatabase _instance = new SessionManagerDatabase();
    public static SessionManagerDatabase GetInstance()
    {
        return _instance;
    }

    public SessionManagerDatabase()
    {
        Console.WriteLine($"{nameof(SessionManagerDatabase)} constructor called");
        Synchronize();
    }          

    public void Synchronize()
    {
        Console.WriteLine($"{nameof(Synchronize)} called");
        // NullReferenceException here
        List<SessionInfo> memorySessions = SessionManager.GetInstance().GetAllActiveSessions();  
        //...
    }

    public List<SessionInfo> LoadActiveSessionsFromDb()
    {
        Console.WriteLine($"{nameof(LoadActiveSessionsFromDb)} called");
        return new List<SessionInfo>();
    }
}

public class SessionInfo
{
}

The problem still remains if you uncomment the static constructors as suggested in the other question. Use this code to get a TypeInitializationException with the NullRefernceException as InnerException in Synchronize at SessionManager.GetInstance().GetAllActiveSessions():

static void Main(string[] args)
{
    try
    {
        var sessionManagerInstance = SessionManager.GetInstance();
    }
    catch (TypeInitializationException e)
    {
        Console.WriteLine(e);
        throw;
    }
}

Console output:

SessionManager constructor called
RecoverState called
SessionManagerDatabase constructor called
Synchronize called
System.TypeInitializationException: Der Typeninitialisierer für "SessionManager" hat eine Ausnahme verursacht. ---> System.TypeInitializationException: Der Typeninitialisierer für "SessionManagerDatabase" hat eine Ausnahme verursacht. ---> System.NullReferenceException: Der Objektverweis wurde nicht auf eine Objektinstanz festgelegt.
   bei ConsoleApplication_CSharp.Program.SessionManagerDatabase.Synchronize() in ......
   bei ConsoleApplication_CSharp.Program.SessionManagerDatabase..ctor() in ......
   bei ConsoleApplication_CSharp.Program.SessionManagerDatabase..cctor() in ......
   --- Ende der internen Ausnahmestapelüberwachung ---
   bei ConsoleApplication_CSharp.Program.SessionManagerDatabase.GetInstance()
   bei ConsoleApplication_CSharp.Program.SessionManager.RecoverState() in ......
   bei ConsoleApplication_CSharp.Program.SessionManager..ctor() in .....
   bei ConsoleApplication_CSharp.Program.SessionManager..cctor() in ......
   --- Ende der internen Ausnahmestapelüberwachung ---
   bei ConsoleApplication_CSharp.Program.SessionManager.GetInstance()
   bei ConsoleApplication_CSharp.Program.Main(String[] args) in ......

I understand that there is some kind of circular dependency here(in original code not so obvious), but I still don't understand why the code fails to initialize the singletons. What would be the best approach for this use case apart from avoiding circular dependencies?

Community
  • 1
  • 1
Tim Schmelter
  • 450,073
  • 74
  • 686
  • 939
  • 1
    It is odd, I would have expected to see `StackOverflowException`. It's generally not a good idea to do anything too sophisticated during constructors, especially when each is a singleton and each ultimately calls the other before either has fully completed initialing. Perhaps look into _deferred initialisation_? –  Jan 30 '17 at 13:55
  • 4
    If you look this: `private static SessionManager _instance = new SessionManager()`, it has two important steps. 1.- Initialization (`new SessionManager()`) and 2 the asignation(`_instance = the obj`). if you try to use `_instance` before the asignation (as you do), it's null. And It thows your NPE. – David Pérez Cabrera Jan 30 '17 at 13:55

4 Answers4

6

Look at the IL:

IL_0001:  newobj     instance void SO.Program/SessionManager::.ctor()
IL_0006:  stsfld     class SO.Program/SessionManager SO.Program/SessionManager::_instance

Here you see that the call to the static constructor has two steps. It first initializes a new instance, and then it assigns it. That means that when you do cross-class calls which depend on the existence of the instance, you are stuck. It is still in the middle of creating the instance. After that it can be called.

You might get out of this by creating a static Initialize method, which does the instantation calls.

Try this:

static SessionManager()
{
    _instance = new SessionManager();

    _instance.RecoverState();
}

static SessionManagerDatabase()
{
    _instance = new SessionManagerDatabase();

    _instance.Synchronize();
}
Patrick Hofman
  • 153,850
  • 22
  • 249
  • 325
3

You're rushing the steps and you have some sort of recursion going on:

  1. SessionManager _instance = new SessionManager(); this line calls some methods which ends with a call to SessionManagerDatabase.GetInstance()

  2. This also does the same and ends with a call back to SessionManager.GetInstance()

  3. This causes the problem since it requires a valid value held in the _instance variable in SessionManager, but at that point you haven't really finished the chain of method calls in order to give proper value to _instance thus causing NullReferenceException.

Deadzone
  • 793
  • 1
  • 11
  • 33
  • Thanks for the answer. You're right, i noticed it already. But the question still remains how to avoid this circular dependency. +1 – Tim Schmelter Jan 30 '17 at 14:11
  • @TimSchmelter You might want to rethink your design. The usual fix for circular dependency is to use interfaces. It's hard to suggest a different approach working with interfaces as I suppose you have a lot of code that you haven't shown. – Deadzone Jan 30 '17 at 14:30
  • of course you're right, there are other ways to avoid this. But actually Patrick and David have already showed how to fix this easily. Remove the dependency from the constructor and call the method which references the other class from the static constructor. Since only the singleton is used there is no need to call it from the instance constructor. – Tim Schmelter Jan 30 '17 at 14:37
  • Sounds like a good idea, glad you got some fast and accurate responses @TimSchmelter :) – Deadzone Jan 30 '17 at 14:38
2

What happens in your example is that the instance constructor is called during the static field initialization as per specification. But the constructor fails with NullReferenceExeption because it tries to obtain reference to _instance by the GetInstance() call. Please note that the _instance isn't initialized yet - the initialization process is in the progress. Consequently the instance constructor fails because of problem above and therefore it doesn't construct/initialize the _instance field. So briefly you should try to get the static _instance from your instance constructor.

Dmytro Mukalov
  • 1,949
  • 1
  • 9
  • 14
  • Thanks for the answer. You're right, i noticed it already. But the question still remains how to avoid this circular dependency. I don't understand _"try to get the static _instance from your instance constructor"_ +1 – Tim Schmelter Jan 30 '17 at 14:12
  • Sorry I wanted to say you shouldn't try. As for the solution I'd suggest to use Lazy instead of the singleton basing on a static field. The Lazy guarantees thread safe singleton execution of the initialization function where you can obtain all sessions information from the db. – Dmytro Mukalov Jan 30 '17 at 14:32
2

If you look this: private static SessionManager _instance = new SessionManager(), it has two important steps. 

1.- Initialization (new SessionManager()). 
2.- The asignation(_instance = the obj).

If you try to use _instance before the asignation (as you do), it's null. And It thows your NRE. You can break this back call spliting the constructor behaviour like this:

public class SessionManager
{
    private static SessionManager _instance;

    static SessionManager() { 
         _instance = new SessionManager();
         _instance.RecoverState();
    }

    public static SessionManager GetInstance()
    {
        return _instance;
    }

    public SessionManager()
    {
        Console.WriteLine($"{nameof(SessionManager)} constructor called");
        // remove RecoverState() call
    }
David Pérez Cabrera
  • 4,960
  • 2
  • 23
  • 37