The VAR Scope Structs Back

Just when we thought we had conquered it, the VAR scope came back to haunt us.

The Problem

We installed TeaLeaf to improve the usability of our website .outbound}. While mining the error reports, we found an error that was happening nearly a dozen times a week and had never been reported to our help desk.

We have a Reports application written with Mach-II where Insurance Agents can select from a list and generate HTML or XLS reports on the fly.

The Diagnosis

1. What is the error message?

Message Cannot insert item with key claimsReported.
Detail This key already exists.

2. In which file did this happen?

The error was occuring inside a Mach-II Listener.

So what do we know about the Mach-II Listener?

1. It’s a ColdFusion Component (CFC) file.
2. It’s setup in the mach-ii.xml file.
3. It’s a singleton.
4. When it’s created, it’s cached in the application scope.

3. Which function is throwing the error?

ReportListener.cfc

<cffunction name="getReports" access="public" returntype="void" output="true">
  <cfargument name="event" type="MachII.framework.Event" required="true" />

  <cfset var listItem = "" />
  <cfset var counter = 0 />

  <cfset variables.reports = structNew() />

  <cfloop list="#arguments.event.getArg('reportNames')#" index="listItem">

    <cfset counter = counter + 1 />

    <cfset structInsert( variables.reports, listItem, getReport( arguments.event ) ) />
  </cfloop>

  <cfset arguments.event.setArg( "displayReports" , variables.reports)/>

</cffunction>

Can you see the error? Does it just jump out at you?

4. Why is reports in the variables scope? Shouldn’t it be var scoped?

If the error didn’t jump out at you, then read up on the VAR scope.

If I hadn’t been writing that series, this would not have been the next step in my thought process. The rest of my thought process would have been:

• What does this function do?
• Where does the data come from?
• Where does the data go?
• yadda
• yadda
• lunch
• yadda

Thankfully, I was able to skip those steps and realized where the error lie.

What do we know about CFCs placed in the application scope?

1. You should var scope variables specific to a function.
2. Anything in its internal variables scope is available to every function in it.
3. Anything in its internal variables scope is available to and can be manipulated by every thread that calls it.

The Tip-off

Now that we know the solution, what’s more interesting is how we were even able to find the error. When we overhauled the site, one of the Coding Standards I asked everyone to follow was to use Struct Functions as much as possible instead of shortcutting through the code.

The error occured on this line:

<cfset structInsert( variables.reports, listItem, getReport( arguments.event ) ) />

The message was “Cannot insert item with key claimsReported.”. claimsReported is the name of one of the reports we list.

The message detail was “This key already exists”, which means that the code was trying to add a key to the struct variables.reports that was already in the struct.

But the list we send in always has unique values, so there’s no way that this error should occur. Right?

Wrong.

What had been occuring is that two or more report requests would happen concurrently and the value of variables.reports would be manipulated by all of the threads at once.

Thread A asks for “claimsReported,newPolicies,reportE,reportG”.
Thread B asks for “cancelledPolicies,claimsReported,reportB,reportC,reportD”
Thread C asks for “newPolicies,reportA,reportD,reportH”

The value of variables.reports would be changed constantly as each thread entered the function and began to loop over its list of reports.

By the time Thread A completed its loop of 4 list items, it’s possible that Thread B had started looping over its list, so Thread A could get back a struct with more or less than 4 keys in it.

Innacurate reports returned for Thread A

Recreating the error

Let’s create a simple function that loops over a list and turns each list element into a key of a struct.

struct_test.cfm

<cffunction name="structFromList" access="public" output="false" returntype="struct" hint="I create struct keys from list elements.">
  <cfargument name="myList" required="true" type="string" />

  <cfset var x = 0 />
  <cfset var counter = 1 />
  <cfset var reports = structNew() />

  <cfloop list="#arguments.myList#" index="x">

    <cfset structInsert( reports, x, counter ) />
    <cfset counter = counter + 1 />

  </cfloop>

  <cfreturn reports />

</cffunction>

<cfset theList = "a,b,c,d,e,f,g" />

<cfdump var="#structFromList(theList)#" label="theList">

Output

Now let’s try and cause the same error. We need to try and insert a struct key that already exists.

struct_test.cfm - Error

<cfloop list="#arguments.myList#" index="x">

  <cfset structInsert( reports, x, counter ) />
  <cfset structInsert( reports, x, counter + 2 ) />
  <cfset counter = counter + 1 />

</cfloop>

Output

Cannot insert item with key a.

This key already exists.

The error occurred in C:\Inetpub\wwwroot_testing\structs\struct_test.cfm: line 28

26 :
27 :     <cfset structInsert(reports, x, counter) />
28 :     <cfset structInsert(reports, x, counter + 2) />
29 :     <cfset counter = counter + 1 />
30 :

StructInsert requires the struct you’re using, which key to create and what its value will be. The optional attribute is allowoverwrite, which is defaulted to false. This is why the error was thrown.

So what would have happened if we hadn’t used StructInsert? What if we had used an alternate Struct syntax?

structName["structKey"]

struct_test.cfm with alternate syntax

<cfloop list="#arguments.myList#" index="x">

  <cfset reports[x] = counter />
  <cfset reports[x] = counter + 2 />

  <cfset counter = counter + 1 />

</cfloop>

Output

See that? No error.

ColdFusion just overwrites the value of reports["a"] (a.k.a. reports.a), no questions asked.

The Solution

So thankfully, this was an easy fix.

Corrected Function

<cffunction name="getReports" access="public" returntype="void" output="true">
  <cfargument name="event" type="MachII.framework.Event" required="true" />

  <cfset var listItem = "" />
  <cfset var counter = 0 />
  <cfset var reports = structNew() />

  <cfloop list="#arguments.event.getArg('reportNames')#" index="listItem">

    <cfset counter = counter + 1 />

    <cfset structInsert( reports, listItem, getReport( arguments.event ) ) />
  </cfloop>

  <cfset arguments.event.setArg( "displayReports" , reports)/>

</cffunction>

Now each thread gets its own instance of reports to manipulate. The method is now thread safe.

TeaLeaf & Coding Standards Return on Investment (ROI)

FWIW, we had run Mike Schierberl’s varScoper tool over our entire code base before we launched the new site. While it finds unscoped variables that should be in the var scope (unscoped variables are placed in the variables scope by default), it doesn’t find explicitly variables scoped variables that should be in the var scope.

We asked the help desk if there had been any errors reported for the Agent Reports section that didn’t seem to make any sense. They had been getting calls from Agents saying that from time to time, the reports they selected didn’t match the reports they would receive. If they went back and selected the list again, they would get what they requested. It happened so infequently that they never filed a ticket for it.

While the impact of this syntax error was pretty small, in some of our other applications, it could have been critical.

Had we not been using a tool like TeaLeaf, we couldn’t have been so proactive in finding unreported errors.

Had we not been using StructInsert, and Struct Functions in general, I doubt we would have ever figured out why this was happening to our Agents.

Sometimes being verbose with your code can be worth much more than the few seconds you would save otherwise. I know some of you newer developers are thinking, “No. That’s not true. That’s impossible!”

Search your feelings, you know it to be true!