views:

54

answers:

2

I have the code:

internal static class IdCounter
{
    [ThreadStatic]
    private static int _id = 0;

    static IdCounter()
    {

    }

    public static int Id
    {
        get
        {
            lock(typeof(IdCounter))
            {
                return _id++;
            }
        }
    }
}

public abstract class Request
{
    protected Request(int requestId)
    {
        RequestId = IdCounter.Id;
    }
}

On the second call, i receive RequestId equals 2, not 1, where is the problem? I tried to use Thread.SetData, but result is the same.

+5  A: 

(Earlier response removed; I think it may be a red herring.)

In response to the somewhat cryptic comment, do you mean that on the first request you'd expect to get 0, and on the second request get 1? If so, I fail to see how that's related to _ThreadStatic at all. I don't believe that RequestId will return 2 after just two requests, even on the same thread, unless you're seeing strange debugger effects. If you think you're seeing some odd behaviour, please post a short but complete program demonstrating the problem.

Note that as you have a property which increments a counter, if you're stepping through in the debugger it could be evaluating the property when you don't expect it to - even if you're not explicitly looking at the Id property. That could explain what's going on. Try to reproduce the problem without using a debugger.

Ideally, write idempotent properties - if you write properties which have side-effects, you'll always be at risk of oddities when debugging, as well as subtle bugs in code (where you might refactor two property accesses to one, unaware of the side-effects).

Jon Skeet
Check code more careful, before posting a comment.
INs
@INs: Um, fancy being any more cryptic? If you're unclear when asking the question and responding to answers, it will take longer to help you.
Jon Skeet
+3  A: 

Are you checking the value of IdCounter.Id in the debugger? This will evaluate the property, which will increment it.

Modifying operations in property gets are a bad idea.

Also:

  • Using a type instance for locking is a bad idea (the guidelines recommend against it as it can cause problems in some scenarios), instead allocate an instance of Object to lock on.
  • If _id is ThreadStatic each thread has its own instance, so there is no need to lock anyway.
Richard
i check id in proeprty getter, so it was called only once. Also, it is experimental code, just provide why id is not equal to 1.
INs
@INs: Just because you're explicitly *looking* at `id` doesn't mean that the debugger isn't *inspecting* `Id`.
Jon Skeet
@Ins: Please be polite, you're getting answers for /free/. You made two calls: one in the debugger (by looking at `idCounter` to open it to look at the field) and once to get an id.
Richard