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:
- ColdFISH gets re-factored to use only locally varred variables to format code.
- BlogCFC creates a NEW instance of the ColdFISH component for every request.
- 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. :)
2 ********************************************************************************
3 Copyright 2005-2008 ColdBox Framework by Luis Majano and Ortus Solutions, Corp
4 www.coldboxframework.com | www.luismajano.com | www.ortussolutions.com
5 ********************************************************************************
6
7 Author : Luis Majano
8 Date : 10/10/2007
9 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>

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.
Jason