BlogCFC Code Formatting Not Thread Safe (With Example)

I found an interesting little bug in the BlogCFC implementation of ColdFISH today. ColdFISH is a ColdFusion code formatting component that is instantiated once and cached as a singleton in the application scope in BlogCFC. The problem is, ColdFISH looks like it wasn't intended to be used as a singleton. It makes use of the variables scope to store the Java StringBuffer class it uses to gather up your formatted code as well as a number of other variables used to parse the code it is formatting. This means when two or more people hit a BlogCFC entry with larger code samples, race conditions exists.

I see three ways around this:

  1. ColdFISH gets re-factored to use only locally varred variables to format code.
  2. BlogCFC creates a NEW instance of the ColdFISH component for every request.
  3. BlogCFC forces all code formatting to be single-threaded by useing named locks around all calls to ColdFISH.

Personally, I like option 1, but I don't know what Jason's intent was for ColdFish. His documentation doesn't seem to address instantiation and persistence of the component.

I have filed bug reports with both riaforge projects so hopefully one or the other will add a fix into their next version.

For your perverse viewing pleasure, I have included a rather large block of code from the ColdBox framework below. Refresh this page a few times very quickly and scroll through the code paying close attention the line numbers (which will probably be very much out of order). There's also a good chance anyone else hitting an entirely different entry on my site at the same time as you is probably getting a really jacked up code sample to. :)

 <!-----------------------------------------------------------------------
 ********************************************************************************
 Copyright 2005-2008 ColdBox Framework by Luis Majano and Ortus Solutions, Corp
 www.coldboxframework.com | www.luismajano.com | www.ortussolutions.com
 ********************************************************************************
 
 Author :    Luis Majano
 Date :    10/10/2007
 Description :
10      This is the base component used to provide Application.cfc support.
11  ----------------------------------------------------------------------->

12  <cfcomponent name="coldbox" hint="This is the base component used to provide Application.cfc support" output="false">
13  
14  <!------------------------------------------- CONSTRUCTOR ------------------------------------------->
15  
16      <!--- Constructor --->
17      <cfparam name="variables.COLDBOX_CONFIG_FILE"     default="" type="string">
18      <cfparam name="variables.COLDBOX_APP_ROOT_PATH" default="#getDirectoryFromPath(getbaseTemplatePath())#" type="string">
19      
20      <cfscript>
21          instance = structnew();
22          //Set the timeout
23  
        setLockTimeout(30);
24          //Set the app hash
25  
        setAppHash(hash(getBaseTemplatePath()));
26          
27          //Set the COLDBOX CONFIG FILE
28  
        setCOLDBOX_CONFIG_FILE(COLDBOX_CONFIG_FILE);
29          //Set the Root
30  
        setCOLDBOX_APP_ROOT_PATH(COLDBOX_APP_ROOT_PATH);
31      
</cfscript>
32  
33  <!------------------------------------------- PUBLIC ------------------------------------------->
34  
35      <!--- Init --->
36      <cffunction name="init" access="public" returntype="coldbox" hint="Used when not using inheritance" output="false" >
37          <cfargument name="COLDBOX_CONFIG_FILE"      required="true" type="string" hint="The coldbox config file from the application.cfc">
38          <cfargument name="COLDBOX_APP_ROOT_PATH" required="true" type="string" hint="The coldbox app root path from the application.cfc">
39          <cfscript>
40              /* Set vars for two main locations */
41              setCOLDBOX_CONFIG_FILE(arguments.COLDBOX_CONFIG_FILE);
42              setCOLDBOX_APP_ROOT_PATH(arguments.COLDBOX_APP_ROOT_PATH);
43              return this;
44          
</cfscript>
45      </cffunction>
46  
47      <!--- Load ColdBox --->
48      <cffunction name="loadColdbox" access="public" returntype="void" hint="Load the framework" output="false" >
49          <!--- Clean up If Necessary --->
50          <cfif structkeyExists(application,"cbController")>
51              <cfset structDelete(application,"cbController")>
52          </cfif>
53          <!--- Create Brand New Controller --->
54          <cfset application.cbController = CreateObject("component","coldbox.system.controller").init(COLDBOX_APP_ROOT_PATH)>
55          <!--- Setup the Framework And Application --->
56          <cfset application.cbController.getLoaderService().setupCalls(COLDBOX_CONFIG_FILE)>
57      </cffunction>
58      
59      <!--- Reload Checks --->
60      <cffunction name="reloadChecks" access="public" returntype="void" hint="Reload checks and reload settings." output="false" >
61          <cfset var ExceptionService = "">
62          <cfset var ExceptionBean = "">
63          
64          <!--- Initialize the Controller If Needed--->
65          <cfif not structkeyExists(application,"cbController") or not application.cbController.getColdboxInitiated() or isfwReinit()>
66              <cflock type="exclusive" name="#getAppHash()#" timeout="#getLockTimeout()#" throwontimeout="true">
67                  <cfif not structkeyExists(application,"cbController") or not application.cbController.getColdboxInitiated() or isfwReinit()>
68                      <cfset loadColdBox()>
69                      <cfset application.cbController.getPlugin("logger").logEntry("information", "Application #application.cbController.getSetting('AppName')# Reinitialized", "")>
70                      <cfset application.cbController.getPlugin("MessageBox").setMessage("info","Application #application.cbController.getSetting('AppName')# Reinitialized on server #server.hostName#")>
71                  </cfif>
72              </cflock>
73          <cfelse>
74              <cftry>
75                  <!--- AutoReload Tests --->
76                  <cfif application.cbController.getSetting("ConfigAutoReload")>
77                      <cflock type="exclusive" name="#getAppHash()#" timeout="#getLockTimeout()#" throwontimeout="true">
78                          <cfset application.cbController.setAppStartHandlerFired(false)>
79                          <cfset application.cbController.getLoaderService().setupCalls(COLDBOX_CONFIG_FILE)>
80                      </cflock>
81                  <cfelse>
82                      <!--- Handler's Index Auto Reload --->
83                      <cfif application.cbController.getSetting("HandlersIndexAutoReload")>
84                          <cflock type="exclusive" name="#getAppHash()#" timeout="#getLockTimeout()#" throwontimeout="true">
85                              <cfset application.cbController.getHandlerService().registerHandlers()>
86                          </cflock>
87                      </cfif>
88                      <!--- IOC Framework Reload --->
89                      <cfif application.cbController.getSetting("IOCFrameworkReload")>
90                          <cflock type="exclusive" name="#getAppHash()#" timeout="#getLockTimeout()#" throwontimeout="true">
91                              <cfset application.cbController.getPlugin("ioc").configure()>
92                          </cflock>
93                      </cfif>
94                  </cfif>
95                  
96                  <!--- Trap Framework Errors --->
97                  <cfcatch type="any">
98                      <cfset ExceptionService = application.cbController.getExceptionService()>
99                      <cfset ExceptionBean = ExceptionService.ExceptionHandler(cfcatch,"framework","Framework Initialization/Configuration Exception")>
100                      <cfoutput>#ExceptionService.renderBugReport(ExceptionBean)#</cfoutput>
101                      <cfabort>
102                  </cfcatch>
103              </cftry>
104          </cfif>
105      </cffunction>
106      
107      <!--- Process A ColdBox Request --->
108      <cffunction name="processColdBoxRequest" access="public" returntype="void" hint="Process a Coldbox Request" output="true" >
109          <cfset var cbController = 0>
110          <cfset var Event = 0>
111          <cfset var ExceptionService = 0>
112          <cfset var ExceptionBean = 0>
113          <cfset var renderedContent = "">
114          <cfset var eventCacheEntry = 0>
115          <cfset var interceptorData = structnew()>
116          
117          <!--- Start Application Requests --->
118          <cflock type="readonly" name="#getAppHash()#" timeout="#getLockTimeout()#" throwontimeout="true">
119              <cftry>
120                  <!--- set request time --->
121                  <cfset request.fwExecTime = getTickCount()>
122                  <!--- Local Reference --->
123                  <cfset cbController = application.cbController>
124                  <!--- Create Request Context & Capture Request --->
125                  <cfset Event = cbController.getRequestService().requestCapture()>
126                  
127                  <!--- Debugging Monitors & Commands Check --->
128                  <cfif cbController.getDebuggerService().getDebugMode()>
129                      <!--- Commands Check --->
130                      <cfset coldboxCommands(cbController,event)>
131                      <!--- Which panel to render --->
132                      <cfif event.getValue("debugPanel","") eq "cache">
133                          <cfoutput>#cbController.getDebuggerService().renderCachePanel()#</cfoutput>
134                          <cfabort>
135                      <cfelseif event.getValue("debugPanel","") eq "cacheviewer">
136                          <cfoutput>#cbController.getDebuggerService().renderCacheDumper()#</cfoutput>
137                          <cfabort>
138                      <cfelseif event.getValue("debugPanel","") eq "profiler">
139                          <cfoutput>#cbController.getDebuggerService().renderProfiler()#</cfoutput>
140                          <cfabort>
141                      </cfif>                    
142                  </cfif>
143              
144                  <!--- Application Start Handler --->
145                  <cfif cbController.getSetting("ApplicationStartHandler") neq "" and (not cbController.getAppStartHandlerFired())>
146                      <cfset cbController.runEvent(cbController.getSetting("ApplicationStartHandler"),true)>
147                      <cfset cbController.setAppStartHandlerFired(true)>
148                  </cfif>
149                  
150                  <!--- Execute preProcess Interception --->
151                  <cfset cbController.getInterceptorService().processState("preProcess")>
152                  
153                  <!--- IF Found in config, run onRequestStart Handler --->
154                  <cfif cbController.getSetting("RequestStartHandler") neq "">
155                      <cfset cbController.runEvent(cbController.getSetting("RequestStartHandler"),true)>
156                  </cfif>
157                  
158                  <!--- Before Any Execution, do we have cached content to deliver --->
159                  <cfif Event.isEventCacheable() and cbController.getColdboxOCM().lookup(Event.getEventCacheableEntry())>
160                      <cfset renderedContent = cbController.getColdboxOCM().get(Event.getEventCacheableEntry())>
161                      <cfoutput>#renderedContent#</cfoutput>
162                  <cfelse>
163                  
164                      <!--- Run Default/Set Event --->
165                      <cfset cbController.runEvent(default=true)>
166                      
167                      <!--- No Render Test --->
168                      <cfif not event.isNoRender()>
169                          <!--- Check for Marshalling and data render --->
170                          <cfif isStruct(event.getRenderData()) and not structisEmpty(event.getRenderData())>
171                              <cfset renderedContent = cbController.getPlugin("Utilities").marshallData(argumentCollection=event.getRenderData())>
172                          <cfelse>
173                              <!--- Render Layout/View pair via set variable to eliminate whitespace--->
174                              <cfset renderedContent = cbController.getPlugin("renderer").renderLayout()>
175                          </cfif>
176                          
177                          <!--- PreRender Data:--->
178                          <cfset interceptorData.renderedContent = renderedContent>
179                          <!--- Execute preRender Interception --->
180                          <cfset cbController.getInterceptorService().processState("preRender",interceptorData)>
181                          
182                          <!--- Check if caching the content --->
183                          <cfif event.isEventCacheable()>
184                              <cfset eventCacheEntry = Event.getEventCacheableEntry()>
185                              <!--- Cache the content of the event --->
186                              <cfset cbController.getColdboxOCM().set(eventCacheEntry.cacheKey,
187                                                                      renderedContent,
188                                                                      eventCacheEntry.timeout,
189                                                                      eventCacheEntry.lastAccessTimeout)
>

190                          </cfif>
191                          
192                          <!--- Render Content Type if using Render Data --->
193                          <cfif isStruct(event.getRenderData()) and not structisEmpty(event.getRenderData())>
194                              <!--- Render the Data Content Type --->
195                              <cfcontent type="#event.getRenderData().contentType#" reset="true">
196                              <!--- Remove panels --->
197                              <cfsetting showdebugoutput="false">
198                              <cfset event.showDebugPanel(false)>
199                          </cfif>
200                          
201                          <!--- Render the Content --->
202                          <cfoutput>#renderedContent#</cfoutput>
203                              
204                          <!--- Execute postRender Interception --->
205                          <cfset cbController.getInterceptorService().processState("postRender")>
206                      </cfif>
207                      
208                      <!--- If Found in config, run onRequestEnd Handler --->
209                      <cfif cbController.getSetting("RequestEndHandler") neq "">
210                          <cfset cbController.runEvent(cbController.getSetting("RequestEndHandler"),true)>
211                      </cfif>
212                      
213                      <!--- Execute postProcess Interception --->
214                      <cfset cbController.getInterceptorService().processState("postProcess")>
215                  
216                  <!--- End else if not cached event --->
217                  </cfif>
218                  
219                  <!--- Trap Application Errors --->
220                  <cfcatch type="any">
221                      <!--- Get Exception Service --->
222                      <cfset ExceptionService = cbController.getExceptionService()>
223                      
224                      <!--- Intercept The Exception --->
225                      <cfset interceptorData = structnew()>
226                      <cfset interceptorData.exception = cfcatch>
227                      <cfset cbController.getInterceptorService().processState("onException",interceptorData)>
228                      
229                      <!--- Handle The Exception --->
230                      <cfset ExceptionBean = ExceptionService.ExceptionHandler(cfcatch,"application","Application Execution Exception")>
231                      
232                      <!--- Render The Exception --->
233                      <cfoutput>#ExceptionService.renderBugReport(ExceptionBean)#</cfoutput>
234                  </cfcatch>
235              </cftry>
236              
237              <!--- DebugMode Routines --->
238              <cfif cbController.getDebuggerService().getDebugMode()>
239                  <!--- Request Profilers --->
240                  <cfif cbController.getDebuggerService().getDebuggerConfigBean().getPersistentRequestProfiler() and
241                       structKeyExists(request,"debugTimers")>

242                      <cfset cbController.getDebuggerService().pushProfiler(request.DebugTimers)>
243                  </cfif>
244                  <!--- Render DebugPanel --->
245                  <cfif Event.getdebugpanelFlag()>
246                      <!--- Time the request --->
247                      <cfset request.fwExecTime = GetTickCount() - request.fwExecTime>
248                      <!--- Render Debug Log --->
249                      <cfoutput>#cbController.getDebuggerService().renderDebugLog()#</cfoutput>
250                  </cfif>
251              </cfif>
252          </cflock>
253          
254      </cffunction>
255      
256      <!--- Session Start --->
257      <cffunction name="onSessionStart" returnType="void" output="false" hint="An onSessionStart method to use or call from your Application.cfc">
258          <cfset var cbController = "">
259          
260          <cflock type="readonly" name="#getAppHash()#" timeout="#getLockTimeout()#" throwontimeout="true">
261          <cfscript>
262              //Setup the local controller
263  
            cbController = application.cbController;
264              
265              //Execute Session Start interceptors
266  
            cbController.getInterceptorService().processState("sessionStart",session);
267              
268              //Execute Session Start Handler
269  
            if ( cbController.getSetting("SessionStartHandler") neq "" ){
270                  cbController.runEvent(cbController.getSetting("SessionStartHandler"),true);
271              }
272          
</cfscript>
273          </cflock>
274      </cffunction>
275      
276      <!--- Session End --->
277      <cffunction name="onSessionEnd" returnType="void" output="false" hint="An onSessionEnd method to use or call from your Application.cfc">
278          <!--- ************************************************************* --->
279          <cfargument name="sessionScope" type="struct" required="true">
280          <cfargument name="appScope"     type="struct" required="false">
281          <!--- ************************************************************* --->
282          <cfset var cbController = "">
283          <cfset var event = "">
284          
285          <cflock type="readonly" name="#getAppHash()#" timeout="#getLockTimeout()#" throwontimeout="true">
286          <cfscript>
287              //Check for cb Controller
288  
            if ( structKeyExists(arguments.appScope,"cbController") ){
289                  //setup the controller
290  
                cbController = arguments.appScope.cbController;
291                  event = cbController.getRequestService().getContext();
292                  
293                  //Execute Session End interceptors
294  
                cbController.getInterceptorService().processState("sessionEnd",arguments.sessionScope);
295                  
296                  //Execute Session End Handler
297  
                if ( cbController.getSetting("SessionEndHandler") neq "" ){
298                      //Place session reference on event object
299  
                    event.setValue("sessionReference", arguments.sessionScope);
300                      //Place app reference on event object
301  
                    event.setValue("applicationReference", arguments.appScope);
302                      //Execute the Handler
303  
                    cbController.runEvent(event=cbController.getSetting("SessionEndHandler"),
304                                           prepostExempt=true,
305                                           default=true);
306                  }
307              }
308          
</cfscript>
309          </cflock>
310      </cffunction>
311  
312      <!--- setter COLDBOX CONFIG FILE --->
313      <cffunction name="setCOLDBOX_CONFIG_FILE" access="public" output="false" returntype="void" hint="Set COLDBOX_CONFIG_FILE">
314          <cfargument name="COLDBOX_CONFIG_FILE" type="string" required="true"/>
315          <cfset variables.COLDBOX_CONFIG_FILE = arguments.COLDBOX_CONFIG_FILE/>
316      </cffunction>
317      
318      <!--- setter COLDBOX_APP_ROOT_PATH --->
319      <cffunction name="setCOLDBOX_APP_ROOT_PATH" access="public" output="false" returntype="void" hint="Set COLDBOX_APP_ROOT_PATH">
320          <cfargument name="COLDBOX_APP_ROOT_PATH" type="string" required="true"/>
321          <cfset variables.COLDBOX_APP_ROOT_PATH = arguments.COLDBOX_APP_ROOT_PATH/>
322      </cffunction>
323      
324      <!--- FW needs reinit --->
325      <cffunction name="isfwReinit" access="public" returntype="boolean" hint="Verify if we need to reboot the framework" output="false" >
326          <cfset var reinitPass = "">
327          <cfset var incomingPass = "">
328          
329          <!--- CF Parm Structures just in case. --->
330          <cfparam name="FORM" default="#StructNew()#">
331          <cfparam name="URL"     default="#StructNew()#">
332          
333          <cfscript>
334              /* Check if we have a reinit password at hand. */
335              if ( application.cbController.settingExists("ReinitPassword") ){
336                  reinitPass = application.cbController.getSetting("ReinitPassword");
337              }            
338              /* Verify the reinit key is passed */
339              if ( structKeyExists(url,"fwreinit") or structKeyExists(form,"fwreinit") ){
340                  
341                  /* pass Checks */
342                  if ( reinitPass eq "" ){
343                      return true;
344                  }
345                  else{
346                      /* Get the incoming pass from form or url */
347                      if( structKeyExists(form,"fwreinit") ){
348                          incomingPass = form.fwreinit;
349                      }
350                      else{
351                          incomingPass = url.fwreinit;
352                      }
353                      /* Compare the passwords */
354                      if( Compare(reinitPass, incomingPass) eq 0 ){
355                          return true;
356                      }
357                      else{
358                          return false;
359                      }
360                  }//end if reinitpass neq ""
361  
                
362              }//else if reinit found.
363  
            else{
364                  return false;
365              }
366          
</cfscript>
367      </cffunction>
368      
369  <!------------------------------------------- PRIVATE ------------------------------------------->    
370      
371      <!--- coldboxCommands --->
372      <cffunction name="coldboxCommands" output="false" access="private" returntype="void" hint="Execute some coldbox commands">
373          <cfargument name="cbController" type="any" required="true" default="" hint="The cb Controller"/>
374          <cfargument name="event"         type="any" required="true" hint="An event context object"/>
375          <cfset var command = event.getTrimValue("cbox_command","")>
376          <cfscript>
377              /* Verify it */
378              if( len(command) eq 0 ){ return; }
379              /* Commands */
380              switch(command){
381                  case "expirecache" : {cbController.getColdboxOCM().expireAll();break;}
382                  case "delcacheentry" : {cbController.getColdboxOCM().clearKey(event.getValue('cbox_cacheentry',""));break;}
383                  case "clearallevents" : {cbController.getColdboxOCM().clearAllEvents();break;}
384                  case "clearallviews" : {cbController.getColdboxOCM().clearAllViews();break;}
385                  default: break;
386              }
387          
</cfscript>
388          <!--- Relocate --->
389          <cfif event.getValue("debugPanel","") eq "">
390              <cflocation url="index.cfm" addtoken="false">
391          <cfelse>
392              <cflocation url="index.cfm?debugpanel=#event.getValue('debugPanel','')#" addtoken="false">
393          </cfif>
394      </cffunction>
395      
396      <!--- Getter setter lock timeout --->
397      <cffunction name="getLockTimeout" access="private" output="false" returntype="numeric" hint="Get LockTimeout">
398          <cfreturn instance.LockTimeout/>
399      </cffunction>
400      <cffunction name="setLockTimeout" access="private" output="false" returntype="void" hint="Set LockTimeout">
401          <cfargument name="LockTimeout" type="numeric" required="true"/>
402          <cfset instance.LockTimeout = arguments.LockTimeout/>
403      </cffunction>
404      
405      <!--- AppHash --->
406      <cffunction name="getAppHash" access="private" output="false" returntype="string" hint="Get AppHash">
407          <cfreturn instance.AppHash/>
408      </cffunction>
409      <cffunction name="setAppHash" access="private" output="false" returntype="void" hint="Set AppHash">
410          <cfargument name="AppHash" type="string" required="true"/>
411          <cfset instance.AppHash = arguments.AppHash/>
412      </cffunction>
413  
414  </cfcomponent>

Comments (Comment Moderation is enabled. Your comment will not appear until approved.)
marc esher's Gravatar Hey Brad,
Looking at this code, there ain't a damn thing singleton-safe about it. I agree that your choice #1 is ideal, but that would require a substantial rewrite. I think #2 is the most realistic approach, and Jason should add a big old warning into this component about not using it the way BlogCFC uses it.

Good bug, and a great example of why blindly dropping components into the Application scope is a baaaaad thing.
# Posted By marc esher | 12/4/09 9:06 AM
Jason Delmore's Gravatar I wrote coldfish to be a transient object, but with this and the other requests that came in I decided to do a significant update. I have rewritten coldfish.cfc to be a thread-safe component that can be persisted, and I have written separate components that are spawned for handling formatting and configuration management. I tossed in caching of formatted strings while I was at it (if you're going to persist the object there may as well be some benefit to having it be persisted.) It needs a bit more spit and polish before I release the next version, but it should be a nice update. Coming soon...

Jason
# Posted By Jason Delmore | 12/29/09 10:28 PM
Brad Wood's Gravatar That's awesome, Jason! Thanks for the update. I really like caching idea.
# Posted By Brad Wood | 12/30/09 12:13 PM


BlogCFC was created by Raymond Camden. This blog is running version 5.9.5. Contact Blog Owner