More on threads: AtomicIntegers

G'day:
After commenting on the non-thread-safety of the ++ operator (and its ilk) last week, I decided to look at a solution to this. In the comments of the second article, Justin Carter pointed me to the java.util.concurrent.atomic Java package, within which is an AtomicInteger (Justin actually links to the Java 5 version, but let's stay current with the Java 7 version).


The first thing I did was to grab my sample code from the other day, which was - as a reminder - this:

cfthread.data=[];

request.sequence = 0;    // used to demonstrate which order the threads did the work

for (i=1; i <= 3; i++){
    thread name=i action="run" i=i {
        sleep(randRange(10,20));
        request.sequence++;
        arrayAppend(cfthread.data, "Appended by thread #i# @ sequence #request.sequence#");
        thread.data = cfthread.data;
    }
}

thread action="join" name="1,2,3";

writeDump(cfthread.data);

About 10% of the time, this code will output this sort of thing:

array
1	Appended by thread 3 @ sequence 1
2	Appended by thread 1 @ sequence 2
3	Appended by thread 2 @ sequence 2

Notice how the sequence variable isn't incremented "correctly". This is the root of the issue, and discussed in the other articles.

For my first attempt at fixing this, I simply swapped request.sequence for an AtomicInteger, but otherwise left the logic the same:

cfthread.data=[];

request.sequence = createObject("java", "java.util.concurrent.atomic.AtomicInteger").init();

for (i=1; i <= 3; i++){
    thread name=i action="run" i=i {
        sleep(randRange(10,20));
        request.sequence.incrementAndGet();
        arrayAppend(cfthread.data, "Appended by thread #i# @ sequence #request.sequence.toString()#");
        thread.data = {
            actual        = cfthread.data,
            activity    = cfthread.activity
        };
    }
}

thread action="join" name="1,2,3";

writeDump(cfthread.data);

The effect of doing this had four variations:

1. about 50% of the time the code ran as I would want it to (compared to 90% of the time just using an integer... so this was way worse!);
2. about 5% of the time it exhibited the same mis-sequencing result as the original code;
3. about 15% of  the time I got a 503 server error;
4. and the rest of the time the page never loaded (I'm assuming because there was a deadlock going on).

This was not heartening.

I then augmented my code slightly to see what was going on. I knew enough to just presume I was getting it wrong.

cfthread.data=[];
cfthread.activity = [];

request.sequence = createObject("java", "java.util.concurrent.atomic.AtomicInteger").init();

for (i=1; i <= 3; i++){
    thread name=i action="run" i=i {
        sleep(randRange(10,20));
        arrayAppend(cfthread.activity, "Thread #i# before incrementAndGet(): #request.sequence.toString()#");
        request.sequence.incrementAndGet();
        arrayAppend(cfthread.activity, "Thread #i# after incrementAndGet(): #request.sequence.toString()#");
        arrayAppend(cfthread.data, request.sequence.toString());
        thread.data = {
            actual        = cfthread.data,
            activity    = cfthread.activity
        };
    }
}

thread action="join" name="1,2,3";

writeDump(var={activity=cfthread.activity, actual=cfthread.data});

This gave me more to go on. When things were working, I got this:

struct
ACTIVITY	array 1 Thread 3 before incrementAndGet(): 0 2 Thread 3 after incrementAndGet(): 1 3 Thread 1 before incrementAndGet(): 1 4 Thread 1 after incrementAndGet(): 2 5 Thread 2 before incrementAndGet(): 2 6 Thread 2 after incrementAndGet(): 3
ACTUAL	array 1 1 2 2 3 3

But sometimes I got this:

struct
ACTIVITY	array 1 Thread 2 before incrementAndGet(): 0 2 Thread 3 before incrementAndGet(): 0 3 Thread 2 after incrementAndGet(): 2 4 Thread 3 after incrementAndGet(): 2 5 Thread 1 before incrementAndGet(): 2 6 Thread 1 after incrementAndGet(): 3
ACTUAL	array 1 2 2 2 3 3

Looking at this I slapped my forehead and called myself a dick. Obviously (well: one would have thought it ought to be obvious) I'm causing the same problem I'm actually trying to solve here. Because I have got the increment of the sequence variable and the usage of the sequence variable as two separate statements, I'm opening myself up for this to happen:

1. First thread increments the sequence
2. Second thread increments the sequence
3. First thread outputs it
4. Second thread outputs it

So this is a slightly different version of what was already happening.

Looking more closely at the AtomicInteger method I'm using, the answer is in the method name: incrementAndGet(). What I need to do is to use this method to do the increment and then use what it returns as the value to output. This keeps everything atomic.  So I updated the code to be as follows:

cfthread.data=[];
cfthread.activity = [];

request.sequence = createObject("java", "java.util.concurrent.atomic.AtomicInteger").init();

for (i=1; i <= 3; i++){
    thread name=i action="run" i=i {
        sleep(randRange(10,20));
        arrayAppend(cfthread.activity, "Thread #i# before incrementAndGet(): #request.sequence.toString()#");
        valueToUse = request.sequence.incrementAndGet();
        arrayAppend(cfthread.activity, "Thread #i# after incrementAndGet(): valueToUse: #valueToUse# sequence: #request.sequence.toString()#");
        arrayAppend(cfthread.data, valueToUse);
        thread.data = {
            actual        = cfthread.data,
            activity    = cfthread.activity
        };
    }
}

thread action="join" name="1,2,3";

writeDump(var={activity=cfthread.activity, actual=cfthread.data});

So with this code, even when the threads overlap, the end result is OK:

struct
ACTIVITY	array 1 Thread 3 before incrementAndGet(): 0 2 Thread 3 after incrementAndGet(): valueToUse: 1 sequence: 1 3 Thread 2 before incrementAndGet(): 1 4 Thread 1 before incrementAndGet(): 1 5 Thread 2 after incrementAndGet(): valueToUse: 2 sequence: 3 6 Thread 1 after incrementAndGet(): valueToUse: 3 sequence: 3
ACTUAL	array 1 1 2 2 3 3

See here how the sequence has moved on, but we've already fetched the value we need to use as part of the atomic increment operation, so we always end up with the correct value in the actual array.

What I've learned here is that atomicity is something one needs to give more thought to than simply using a class which implements atomic methods... the method themselves might be atomic, but I need to remember that as soon as one calls a second atomic operation, the first and second operations taken as a whole are not atomic.

All this might be obvious to everyone except me, but it did bite me initially, so perhaps it would bite other people too.

Righto.

--
Adam