How do I check for overflow after an Interlocked.Increment in C#?

Question

What is the correct way to check for an overflow after a call to Interlocked.Increment?

I have an ID generator that generates unique ids during the program's execution, and currently I test if the increment returned zero.

public static class IdGenerator {
    private static int _counter = 0;
    
    public static uint GetNewId() {
        uint newId = (uint)System.Threading.Interlocked.Increment(ref _counter);
        if (newId == 0) {
             throw new System.Exception("Whoops, ran out of identifiers");
        }
        return newId;
    }
}

Given the rather large number of IDs that I generate per run, it is possibile (on an exceptionally large input) that the _counter will overflow when incremented, and I want to throw an exception in that case (crash early to ease debugging).

Excerpt from Microsoft's documentation:

This method handles an overflow condition by wrapping: if location = Int32.MaxValue, location + 1 = Int32.MinValue. No exception is thrown.

@SLaks (your last comment) Well, `(uint)int.MinValue` as a literal won't compile. But a non-const variable or expression `int i = int.MinValue` which hits the conversion `(uint)i` will not throw in the usual `unchecked` context. — Jeppe Stig Nielsen, Jul 22 '13 at 15:30
@SLaks Thanks, I didn't think of that, the cast should be wrapped with `unchecked`, like this: `unchecked((uint)int.MinValue)` I suppose. — Suzanne Soy, Jul 22 '13 at 15:31

SLaks · Accepted Answer · 2013-07-22T15:24:41.383

6

Just check whether newId is Int32.MinValue (before casting to uint) and throw an exception.

The only way to get MinValue from an increment is through overflow.

edited Jul 22 '13 at 15:24

answered Jul 22 '13 at 15:18

SLaks

868,454
176
1,908
1,964

What if two threads incremented the value in short succession? – Jeppe Stig Nielsen Jul 22 '13 at 15:19
2

@JeppeStigNielsen: Then one of them will overflow and the other won't. The whole point of `Interlocked.Increment` is that it's atomic; that isn't a problem. – SLaks Jul 22 '13 at 15:20
Ah, sure, you tell him to check the return value from `Increment`, not the actual value of `_counter` field. I agree with you, then. But he should get rid of his conversion to `uint`, or something. – Jeppe Stig Nielsen Jul 22 '13 at 15:23
@JeppeStigNielsen: `newId` _is_ the returned value from `Increment()`. – SLaks Jul 22 '13 at 15:24
4

You were right, there was no issue there. __Something else:__ As his code stands in the question, he just re-interpretes the 32-bit signed integer as an _unsigned_ 32-bit integer, and he is able to get IDs up to `4'294'967'295`. So his original code worked well, if we assume that he doesn't build his code with `/checked` compiler option. If he uses your answer, he can generate only (roughly) half the IDs he could before. – Jeppe Stig Nielsen Jul 22 '13 at 15:47

score 2 · Answer 2 · answered Sep 13 '16 at 12:22

Consider using unchecked

public static class IdGenerator
{
    private static int _counter;

    public static uint GetNewId()
    {
        uint newId = unchecked ((uint) System.Threading.Interlocked.Increment(ref _counter));
        if (newId == 0)
        {
            throw new System.Exception("Whoops, ran out of identifiers");
        }
        return newId;
    }
}

In this case you get

Little performance boost, because compiler won't check overflow.
x2 space for keys
Simpler and smaller code

How do I check for overflow after an Interlocked.Increment in C#?

2 Answers2