c# Lock with Thread.Sleep not working

Question

I have the following code. In windows server 2008, the program is correct and runs as expected. It outputs 10 different ids.

However, when I run it in windows server 2003, the program is not correct. It outputs 10 ids, but some of the id's are duplicates. It seems that the lock is not working.

If I set Thread.Sleep(500), it works correctly on windows server 2003.

class Test
{
    static void Main(string[] args)
    {
        for (int i = 0; i < 10; i++)
        {
            Console.WriteLine(Util.GetId());
        }
        Console.ReadLine();
    }
}

public class Util
{
    private static object idlock = new object();

    public static string GetId()
    {
        string id = "";
        lock (idlock)
        {
            Thread.Sleep(1);
            id = System.DateTime.Now.ToString("yyMMddHHmmssffff");
        }
        return id;
    }
}

Answer 1

Locking is completely unnecessary; the problem is that DateTime.Now only has a granularity of ~15ms or so (depending on your system). Don't use time as an id in the first place; you could easily just do something like this instead:

public static class Util
{
    static long _id;
    public static string GetId()
    {
        return Next().ToString("0000000000000000");
    }

    private static long Next()
    {
        return Interlocked.Increment(ref _id);
    }
}

Answer 2

Thread.Sleep's parameter is in milliseconds, pick a bigger number. Or use Guid.NewGuid() to generate an actually unique ID without wasting time.

Answer 3

Would DateTime.Ticks work for you? It's got 100-nanosecond resolution, so it sounds like it may work. You won't have to lock anything either, IMO.

Answer 4

While I wold suggest getting away from this method of generating keys, if you are constrained by other conditions then one way you could fix your code without forcing a wait is to keep a reference to the last generated id, then while the just generated id still matches the old one, sleep(10) or so.

This would keep all the existing properties of your code, I think the main one being that it will keep incrementing between runs of the program, not a single session like the other examples presented.

Answer 5

Now that you're locking, try to save off the prior value, and compare for equality. If equals, don't return, but rather Sleep and try again, in a loop.

Personally, I think that this approach sucks. Use GUIDs, they are your friend.