locked
Singleton being destroyed RRS feed

  • Question

  • User-975492262 posted

    I've run into a very illusive problem using a singleton in one of my programs.  It appears that the singleton is being "destroyed."  When it is referenced, a null reference exception is generated.  To validate that this is actually what was going on, I temporarily changed the code so I could create individual instances of the class.  When I did that, there was never a null reference exception.

     Even more strange, this does not happen on all computers.  The problem first became apparently when the software was installed on a client's note book.  This particular machine is working on vista, thought our DEV machines are on vista we are not able to have this problem. And I see the same problem occuring in all Windows 7 machines.  Initially, it had all the security features enabled and some virus protection.  I disabled all of that, but the problem still persisted.

     Does anyone have any ideas on what might be causing this?  The code is posted below.  Thanks for the assistance everyone.

    using System;
    using System.IO;
    using trs.meta;

    namespace trs.meta
    {
     /// <summary>
     /// Singleton class that provides a facility to create
     ///  (highly probable) globally unique row values.
     /// </summary>
     /// <remarks>
     /// Singleton pattern employed here sourced at:
     ///  http://www.yoda.arachsys.com/csharp/singleton.html
     /// </remarks>
     public sealed class RowIdCache
     {
      /// <summary>
      /// Private readonly member variables.
      /// </summary>
            private static readonly RowIdCache _instance = new RowIdCache();
      private readonly object _padlock = new object();

      /// <summary>
      /// Private member variables.
      /// </summary>
      private long _row = 0L;
      private long _lastRowSaved = 0L;
      private long _saveIncrement = 30000L;
      private string _filePath = null;

      /* Explicit static constructor to tell C# compiler
       * not to mark type as beforefieldinit
       */
            static RowIdCache()
            {
              
            }

      private RowIdCache()
      {
       reset();
      }

      /// <summary>
      /// Instance property.
      /// </summary>
      public static RowIdCache Instance
      {
       get { return _instance; }
      }

      /// <summary>
      /// Clear the cache.
      /// </summary>
      public void reset()
      {
       // make sure only one thread is accessing at a time
       lock (_padlock)
       {
        _row = DateTime.Now.Ticks;
        loadRow();
       }
      }

      /// <summary>
      /// Returns the next row value.
      /// </summary>
      /// <returns>long</returns>
      public long getNextRow()
      {
       // make sure only one thread is accessing at a time
       lock (_padlock)
       {
        // increment row value
        _row++;

        // save the row value to disk for every save increment
        if (_row > (_lastRowSaved + _saveIncrement))
        {
         saveLastRow();
        }
      
        return _row;  
       }
      } 

      /// <summary>
      /// Returns the next IRowId value.
      /// </summary>
      /// <returns>IRowId</returns>
      public IRowId getNextRowId()
      {
       return RowId.create(this.getNextRow());  
      } 

      /// <summary>
      /// Determine if the last saved row value is a greater value
      ///  then the current row value. If true then use it otherwise
      ///  save the current row value.
      /// </summary>
      private void loadRow()
      {
       if (null == _filePath)
       {
        // determine the path to the RowIdCache file
        string path = FileShareCache.Instance.getPath(BaseMeta.PATH_ROW_ID);
       
        string filePath = path + @"\" + BaseMeta.FILE_ROW_ID + BaseMeta.FILE_EXT_TEXT;
        _filePath = filePath.ToLower();
       }

       try
       {
        using (StreamReader sr = new StreamReader(_filePath, System.Text.Encoding.Default))
        {
         string sRow = sr.ReadLine();
         if (null != sRow)
         {
          _lastRowSaved = Convert.ToInt64(sRow);
          if (_lastRowSaved > _row)
          {
           // set row to last saved row + the saved increment + 1 to
           // guarantee that the row value has not been used
           _row = _lastRowSaved + _saveIncrement + 1;
           saveLastRow();       
          }
         }
        }

       }
       catch (FileNotFoundException fileNotFound)
       {
        string asExpected = fileNotFound.Message;
        // probably file has not been created
        saveLastRow();
       }
       catch (Exception ex)
       {
        FileLog.Instance.write(ex);
       }
      }    

      /// <summary>
      /// Write the current row value to disk.
      /// </summary>
      private void saveLastRow()
      {
       // write row value fo disk
       _lastRowSaved = _row;

       try
       {
        if (File.Exists(_filePath))
        {
         File.Delete(_filePath);
        }

        using (StreamWriter sw = File.CreateText(_filePath))
        {
         sw.WriteLine(Convert.ToString(_lastRowSaved));
        }
       }
       catch(Exception ex)
       {
        FileLog.Instance.write(ex);
       }
      }
     }
    }

    Wednesday, April 20, 2011 9:46 AM

Answers

  • User-1179452826 posted

    Your implementation of a Singleton is quite flawed. Consider either of the following:

    public class Singleton
    {
        private static Singleton _instance;
        private static object _lock = new object();
    
        private Singleton(){}
        
        public static Singleton Instance
        {
            get
            {
                   if(_instance == null)
                   {
                          lock(_lock){
                                  if(_instance == null)
                                        _instance = new Singleton();
                          }
                   }
       
                   return _instance;
            }
        }
    }

     

     

    OR in c# 4:

    public class Singleton
    {
         private Singleton(){}
         private static readonly Lazy<Singleton> _instance = new Lazy<Singleton>(()=>new Singleton());
    
        public static Singleton Instance{ get {return _instance.Value; } }
    }

    The latter is preferable as there's no double checked lock.

    • Marked as answer by Anonymous Thursday, October 7, 2021 12:00 AM
    Wednesday, April 20, 2011 10:46 AM
  • User-1179452826 posted

    Your singleton code looks ok, so your error must be somewhere else.

    [Also note, when posting code, please use the code tool. It's the icon with green {} right next to the html button in the menu with Bold, Italic etc. button. That should format your code nicely.]

    • Marked as answer by Anonymous Thursday, October 7, 2021 12:00 AM
    Monday, April 25, 2011 6:17 AM
  • User-1011137159 posted

    Below is the good example with code that help to resolve your problem...

    http://amitpatelit.wordpress.com/2011/03/11/singleton-pattern-used-in-window-application/

    • Marked as answer by Anonymous Thursday, October 7, 2021 12:00 AM
    Monday, April 25, 2011 6:21 AM

All replies

  • User-1179452826 posted

    Your implementation of a Singleton is quite flawed. Consider either of the following:

    public class Singleton
    {
        private static Singleton _instance;
        private static object _lock = new object();
    
        private Singleton(){}
        
        public static Singleton Instance
        {
            get
            {
                   if(_instance == null)
                   {
                          lock(_lock){
                                  if(_instance == null)
                                        _instance = new Singleton();
                          }
                   }
       
                   return _instance;
            }
        }
    }

     

     

    OR in c# 4:

    public class Singleton
    {
         private Singleton(){}
         private static readonly Lazy<Singleton> _instance = new Lazy<Singleton>(()=>new Singleton());
    
        public static Singleton Instance{ get {return _instance.Value; } }
    }

    The latter is preferable as there's no double checked lock.

    • Marked as answer by Anonymous Thursday, October 7, 2021 12:00 AM
    Wednesday, April 20, 2011 10:46 AM
  • User-975492262 posted

    Hi Thanks for your response.

    As i am using Dot net 2.0. So the last option is ruled out.

    If i use the first solution I am not able to access the page. Any idea why I am facing this problem.

    After the change my code looks like..

     

    using

    System;

    using

    System.IO;

    using

    trs.meta;

    namespace

    trs.meta

    {

     

    /// <summary>

     

    /// Singleton class that provides a facility to create

     

    /// (highly probable) globally unique row values.

     

    /// </summary>

     

    /// <remarks>

     

    /// Singleton pattern employed here sourced at:

     

    /// http://www.yoda.arachsys.com/csharp/singleton.html

     

    /// </remarks>

     

    public sealed class RowIdCache

    {

     

    /// <summary>

     

    /// Private readonly member variables.

     

    /// </summary>

     

    private static RowIdCache _instance;

     

    private readonly object _padlock = new object();

     

    private static object _lock = new object();

     

    /// <summary>

     

    /// Private member variables.

     

    /// </summary>

     

    private long _row = 0L;

     

    private long _lastRowSaved = 0L;

     

    private long _saveIncrement = 30000L;

     

    private string _filePath = null;

     

    ///* Explicit static constructor to tell C# compiler

     

    // * not to mark type as beforefieldinit

     

    // */

     

    //static RowIdCache()

     

    //{

     

    //}

     

    private RowIdCache()

    {

    reset();

    }

     

    /// <summary>

     

    /// Instance property.

     

    /// </summary>

     

    public

     static RowIdCache Instance

    {

     

    get {

     

    if (_instance == null)

    {

     

    lock (_lock)

    {

     

    if (_instance == null)

    _instance =

    new RowIdCache();

    }

    }

     

    return _instance; }

    }

    Thursday, April 21, 2011 5:30 AM
  • User-1179452826 posted

    Your singleton code looks ok, so your error must be somewhere else.

    [Also note, when posting code, please use the code tool. It's the icon with green {} right next to the html button in the menu with Bold, Italic etc. button. That should format your code nicely.]

    • Marked as answer by Anonymous Thursday, October 7, 2021 12:00 AM
    Monday, April 25, 2011 6:17 AM
  • User-1011137159 posted

    Below is the good example with code that help to resolve your problem...

    http://amitpatelit.wordpress.com/2011/03/11/singleton-pattern-used-in-window-application/

    • Marked as answer by Anonymous Thursday, October 7, 2021 12:00 AM
    Monday, April 25, 2011 6:21 AM