Thread-safety of integer counters in ColdFusion

I was a quite floored this morning when I discovered that Ben Nadel had taken some comments I made regarding thread safety on his blog post on ColdFusion 9 caching and used it to write an entirely new blog post on the AtomicInteger Java object.

In the first post, Ray Camden commented that he thought there might be a race condition in the array loop in Ben's code sample. I later chimed in, expressing my opinion that the race condition was not in the array loop but in the ID generator, and the latter could be fixed with an AtomicInteger object. Ben followed up by saying a named cflock would also work, and I replied that it would interesting to compare the performance of the two techniques.

Any of you who are followers of Ben Nadel's blog know that he is an irredeemable empiricist. He rarely accepts theories on how programs work without first testing them with his own experiments. His intellectual curiosity and honesty makes his blog a joy to read, and I find there is always something to be learned from his experiments.

Apparently my final comment on Ben's caching post caught his attention, as he made it into the topic of a new blog post on AtomicInteger. It was awesome to see him put enough thought into the topic to whip up a performance test and then share his thoughts on the topic and the test results on his blog.

While I enjoyed reading his new blog post, I decided that his experiment needed to be taken a little further. There were two issues that Ben's experiment did not address:

1. The experiment was single-threaded, so it did not test the performance of the counters when shared by multiple threads;
2. The experiment did not compare the thread-safety of the two approaches against the control case (no locking).

Ben had already done the heavy lifting, so all I had to do was add the thread code and the no-locking case. I changed the tests so that each test created 10 threads, and each thread incremented the shared counter 100,000 times. The correct final result of each test would then be 10 * 100,000 = 1,000,000. However if the counter experienced any race conditions, some of the increments would be lost and the final result would be less than 1,000,000.

Since blogger.com does not offer a good way to include long code snippets in a blog post, I decided to publish the code on pastebin. You can check it out on http://cfm.pastebin.com/f18eee642

The performance part of my new test matched Ben's results:

Named CFLOCK Test: 22,807 ms

AtomicInteger Test: 2,403 ms

No-Locking Test: 2,574 ms

I was a little surprised that the no-locking test was slightly slower than the AtomicInteger test. I can only guess that AtomicInteger offers a speed benefit over ColdFusion's ++ operator that outweighs its thread-safety overhead.

The thread-safety part of my new test, on the other hand, completely blew my mind:

Expected final counter value: 1,000,000

Named CFLOCK final counter value: 1,000,000

AtomicInteger final counter value: 1,000,000

No-Locking final counter value: 497,246

The final counter value of the thread-unsafe test was half that of the thread-safe tests. That means that with 10 concurrent threads, approximately 50% of the shared counter increments were lost due to race conditions!!! It turns out experiencing race conditions with the ColdFusion ++ operator was much more likely than what I originally thought. Empirical testing FTW!

Thanks again to Ben Nadel for inspiring me to take this investigative journey and share it with others.

ColdFusion shared scopes and race conditions

In a recent Google Group thread discussing the Singleton design pattern, Phillip Senn asked whether the application scope needs to be locked in onApplicationStart. Ray Camden answered with the following:

You don't need to lock if you let them run "normally." If you run the methods manually (many people will add a call to onApplicationStart inside their onRequestStart if a URL param exist) then you may need a lock. Of course, you only need a lock if you actually care about a race condition. 99% of the code I see in application startup just initializes a set of variables.

Ray's comment about not needing to lock when onApplicationStart is invoked implicitly during the application start event is completely correct. However, things get a little more complicated when you directly invoke the same method. Not only does it create race conditions, but the race conditions it creates are often not easily solved by cflock.

Let me explain with a story. Once upon a time a CF coder wrote some application initialization code that looked like this:

application.settings = {};

application.settings.title = "My application";

application.settings.foo = 42;

application.settings.bar = 60;

application.settings.baz = false;

To allow these settings to be reloaded, the coder added the following to onRequestStart:

<cfif StructKeyExists(url,"init") and url.init eq "abracadabra">

<cfset onApplicationStart() />

</cfif>

This worked fine in development and testing and so was deployed to production.

The application was successful and its usage grew. The original coder moved on to other projects and the maintenance was handed off to a new coder. Many months later the new coder noticed something. Occasionally, when the maintainer used the init=abracadabra parameter to reset the application variables, he would see errors pop up in the error log at the same time. The errors looked something like this:

Element BAR is undefined in APPLICATION.SETTINGS

The source of the error was not isolated to a single place in the code; it occurred in many different places in the application. The element also changed: sometimes it was BAR, other times it was BAZ or FOO or TITLE. The only thing in common to all the errors was APPLICATION.SETTINGS.

The coder couldn't see any pattern to the errors and so wrote it off as an anomaly; either an obscure bug in ColdFusion or some server misconfiguration. The errors continued but not frequently enough for end-users to really notice and complain about it.

Some time later, a project to add enhancements was approved for the application and another coder was brought in to assist the maintainer. The new coder is a bit of a "guru" and knew something about race conditions. When the code guru saw the strange errors he quickly identified the source of the problem: the initialization code contained a race condition.

By initializing the application.settings variable with an empty structure, there was a short period of time when the structure did not contain the title, foo, bar, or baz members. This is not a problem during the application start event because the event occurs before any normal request processing, but it is a problem if the same code invoked directly. If any other request thread tries to access one of those members after the re-init has created the structure but before it has assigned a value to that member, an error will occur.

So how did the guru fix the race condition? The naïve approach would be to simply put a cflock around the call:

<cfif StructKeyExists(url,"init") and url.init eq "abracadabra">

<cflock scope="application" type="exclusive" timeout="60">

<cfset onApplicationStart() />

</cflock>

</cfif>

However, to making this work would also require a read lock around every single access to the application.settings structure! A painful approach to say the least.

The guru knew a better way. He made some simple changes to the initialization code:

var settings = {};

settings.title = "My application";

settings.foo = 42;

settings.bar = 60;

settings.baz = false;

application.settings = settings;

Voila! By initializing the settings structure in a local variable and only assigning it to the application scope after it is fully initialized, the original race condition disappeared. No locking required!

The fix was deployed and the no trace of the error was ever found again.

Some time later, another coder noticed that he was seeing a similar problem in his application, except his errors were in the session scope and they occurred much more frequently.

The CF guru rolled his eyes. "Here we go again," he thought to himself...

Model-Glue: Event result handlers

One feature of Model-Glue that seems to frequently trip newcomers is the <result> tag. I tend to call these result handlers, to distinguish them from results issued by controller methods.

A result handler is used in a event handler to specify which event(s) should be processed after the current event. A named result handler is fired only when a controller issues a result of the same name during the event, while an unnamed or default event handler is "always" fired and does not require an explicit result from a controller (except under certain situations that I will explain in this post).

However, a Model-Glue result handler has two quite distinct behaviours depending on the value of the redirect attribute:

• When redirect="true", processing of the current event is stopped and the event queue is discarded. The framework then issues a to the target event. By default the event state is preserved and is available in the target event, but this can be overridden by adding preserveState="false".
• When redirect="false" or is unset, the target event is added to the event queue. Because an event handler may have more than one <result> tag, an event handler may add more than event to the queue. After all matching <result> tags have been processed, the framework invokes the handler for the next event in the queue.

The basics of the <result> tag is explained in the Section 5 of the Model-Glue Quickstart.

A Model-Glue newcomer is likely to first think of <result> as a kind of GOTO statement and may not realize that multiple events can get queued by an event handler. In a recent post to the Model-Glue mailing list, a developer wondered why the following event handler did not work as expected when an error result was issued:

<event-handler name="signup">
<broadcasts>
<message name="doSignupForm" />
</broadcasts>
<results>
<result do="page.index" />
<result name="error" do="signupform" />
</results>
</event-handler>

What was happening of course is that both "page.index" and "signupform" were being added to the event queue. In this case the developer was seeing the view rendered by page.index, but the final rendering would depend on how the two target event handlers were written.

There are two solutions to this. One solution suggested by another member of the mailing list is to have the controller method issue a success result when there is no error:

<event-handler name="signup">
<broadcasts>
<message name="doSignupForm" />
</broadcasts>
<results>
<result name="success" do="page.index" />
<result name="error" do="signupform" />
</results>
</event-handler>

Another solution is to use redirect="true" to imbue the error result with the expected GOTO-like behaviour:

<event-handler name="signup">
<broadcasts>
<message name="doSignupForm" />
</broadcasts>
<results>
<result do="page.index" />
<result name="error" do="signupform" redirect="true" />
</results>
</event-handler>

One interesting feature of redirect result handlers that I've discovered is that they fire immediately upon a controller method issuing them. This means that any controllers that are waiting to process the current or subsequent message broadcasts in the current event will not be executed.

For example, let's say you wanted to use a message broadcast to log successful signups. One approach would be to add a second broadcast to your signup event:

<event-handler name="signup">
<broadcasts>
<message name="doSignupForm" />
<message name="log"> <argument name="message" value="Signup success" /> </message>
/broadcasts>
<results>
<result do="page.index" />
<result name="error" do="signupform" redirect="true" />
</results>
</event-handler>

If the error result uses redirect="true", this will work as intended because an error result would interrupt the event processing so that the log message would not be broadcast. If the redirect="true" is removed, the log message would be broadcast even on error results.

To get the correct behavior for a non-redirect result you must put the log broadcast in the target event. You may need to define an intermediate event for this. Here is an example:

<event-handler name="signup">
<broadcasts>
<message name="doSignupForm" />
</broadcasts>
<results>
<result name="success" do="signupSuccess" />
<result name="error" do="signupform" />
</results>
</event-handler>

<event-handler name="signupSuccess" access="private">
<broadcasts>
<message name="log">
<argument name="message" value="Signup success" />
</message>
</broadcasts>
<results>
<result do="page.index" />
</results>
</event-handler>


In this case I had to use a named result handler for success so that it would not be fired on error. I also declared the target event handler as private: this is to prevent anyone from triggering the event externally.

For the signupSuccess event handler I would make one more change. If a user signs up successfully and then triggers a browser reload, I do not want to process the signup event again. To prevent this, I could change the final result handler to a redirect:

<event-handler name="signupSuccess" access="private">
<broadcasts>
<message name="log">
<argument name="message" value="Signup success" />
</message>
</broadcasts>
<results>
<result do="page.index" redirect="true" />
</results>
</event-handler>


With this change, a user who successfully signs up would end up at index.cfm?event=page.index instead of index.cfm?event=signup. A browser reload at that point would simply reload the main application page instead of repeating the signup processsing.