2

I have the following class with a nested private class and I would like to assign the value of NID to Context. I have used getters and setters however the value of NID (Sequencer.Context = value;) never gets assigned to ( SeqNode = Node.LoadNode(Context); ). What am I doing wrong?

//Instantiation
Increment.NID = "/Root/Dir/NodetoIncrement";
String getSequence = Convert.ToString(Increment.SID);

// The Class
public static class Increment
{
    //Value of the node Context location in the tree to increment ( Increment.NID )
    public static string NID
    {
        set { Sequencer.Context = value; } //The "/Root/Dir/NodetoIncrement";
    }

    //Get the sequence ID
    public static int SID
    {
        get { return Sequencer.GetSeqId; }
    }

    //Nested sequencer class. This increments the node.

    private class Sequencer
    {   
        private Node SeqNode;
        private static int SequenceNumber;
        private volatile bool Run = true;

        public static string Context { get; set; } //gets 

        public static int GetSeqId
        {
            get
            {
                return Interlocked.Add(ref SequenceNumber, 1);
            }
        }

        public Sequencer() //Constructor Here!
        {
            SeqNode = Node.LoadNode(Context);
            SequenceNumber = Convert.ToInt16(SeqNode["LastSequenceNo"]);

            //NEVER DO THIS .. causes the system to lockup see comments.
            System.Threading.Tasks.Task.Factory.StartNew(() =>
            {
                while (Run)
                {
                    Save();
                    Thread.Sleep(5000);
                }
            });
        }

        private void Save()
        {
            //optimistic concurrency recommended!!
            var retryMaxCount = 3;             // maximum number of attempts
            var cycles = 0;                    // current attempt
            Exception exception = null;        // inner exception storage
            while (cycles++ < retryMaxCount)   // cycle control
            {
                try
                {
                    SeqNode["LastSequenceNo"] = Convert.ToString(SequenceNumber);
                    SeqNode.Save();

                    // successful save, exit loop
                    exception = null;
                    break;
                }
                catch (NodeIsOutOfDateException e)
                {
                    exception = e; // storing the exception temporarily
                    SeqNode = Node.LoadNode(Context);
                }
            }
            // rethrow if needed
            if (exception != null)
                throw new ApplicationException("Node is out of date after 3 attempts.", exception);
        }


        ~Sequencer() { Save(); }

    }
}

public class XYHandler : Node
{
    public override void Save(NodeSaveSettings settings)
    {
        base.Name = Convert.ToString(Increment.SID);
        base.Save();
    }

    public override bool IsContentType
    {
        get { return true; }
    }
}
Xdrone
  • 781
  • 1
  • 11
  • 21

3 Answers3

7

What am I doing wrong?

You are waiting for another thread in a static initializer. NEVER DO THAT. I can't emphasize strongly enough how insanely dangerous that is.

For an explanation why, see this answer:

https://stackoverflow.com/a/8883117/88656

Community
  • 1
  • 1
Eric Lippert
  • 647,829
  • 179
  • 1,238
  • 2,067
  • Thank you for the heads up, but my question now is; what is the best approach to implement threading in this scenario and avoid lockup? – Xdrone Oct 13 '13 at 15:56
  • @xdrone: Just don't go there. A static constructor should initialize some static fields and that's about it. Don't do anything fancy. – Eric Lippert Oct 13 '13 at 18:24
0

Is there some temporal coupling at play here?

SeqNode is set when the Sequencer is instantiated, but I can't see an instantiation in your sample.

The static constructor will run before the property setter is invoked the first time, and then try again 5s later - when is the property being set?

James Holwell
  • 938
  • 7
  • 10
  • Yes temporal coupling is at play here! The node to increment is passed to SeqNode before Sequencer can Increment. "The static constructor will run before the property setter is invoked the first time, and then try again 5s later - when is the property being set?" No the constructor ( SeqNode = Node.LoadNode(Context); ) should fire the Context getter which should return the value of NID. – Xdrone Oct 12 '13 at 00:27
0

I can't see where Sequencer is being constructed (perhaps I missed it). Since it isn't a static constructor it will have to be called at least once for LoadNode to run. Did you intent to make that constructor static also?

Dweeberly
  • 4,668
  • 2
  • 22
  • 41