Reputation: 4768

Static member does not keep assigned value

OK I have a static class that has two static members, a string and a boolean.
A public static method assigns values to these members based upon the state of parameters passed in.

A private static method is then called that processes the static members.

The problem is that while the boolean keeps the value it is set to in the public function, the string does not; it defaults back to its initialised value. Why is this?
Simplified code is below.

static class MessageHandler
{
    private static String m_messageToSend = String.Empty;
    private static bool m_requiresACK = false;


    public static void Send(String message)
    {
         //formatting etc (actual method sets more fields)
         m_messageToSend = message;
         m_requiresACK = true;

         Send();
    }

    private void static Send()
    {
        SendMessageDelegate sendDelegate = DoSend;
        //At this point m_requiresACK remains true but m_messageToSend does not 
        //hold value of message; it is empty.
        IAsyncResult ar = sendDelegate.BeginInvoke(m_messageToSend, m_requiresACK);


        //rest of function
    }
}

//some other class
MessageHandler.Send("Hello");

Upvotes: 0

Answers (5)

Andy_Vulhop

Reputation: 4789

I see in your comments that after that first line, the private variable is empty instead of holding the value of the parameter. Is this true as soon as you hit the first line of Send() ?

Write a unit test or a simple test harness that calls Message.Send("Hello World"); and does an assert on the output. Isolating this from your calling codebase could shine light on whether it is your Message class that is behaving abnormally, or if it is a consumer sending bad/unexpected data.

Also, unless your //rest of function includes resetting your bool, it will always be true after the first message is sent.

Upvotes: 0

Marc Gravell

Reputation: 1062770

You have some huge thread safety issues there. If you really want this static, there is a cheeky fix:

[ThreadStatic]
private static String m_messageToSend = String.Empty;
[ThreadStatic]
private static bool m_requiresACK = false;

This now acts as static, but is per-thread. Crisis averted; but this is a bit... well, I would try to avoid the need, myself - but it will work fine.

important: the initializers are per thread, not per request; since it is likely that your threads will get re-used, you should be sure to initialize the state before trying to use it, or you could have old garbage.

Upvotes: 2

Peter Lillevold

Reputation: 33920

The thread "unsafetyness" of this code could be the problem, since other threads could call Send(string) while your thread is currently in the middle of the same method. I would suggest the following rewrite of the Message class:

static class Message
{
    public static void Send(String message)
    {
         Send(message, true);
    }

    private void static Send(string messageToSend, bool requiresACK)
    {
        SendMessageDelegate sendDelegate = DoSend;
        IAsyncResult ar = sendDelegate.BeginInvoke(messageToSend, requiresACK);

        //rest of function
    }
}

Upvotes: 4