BlogCFC Code Formatting Not Thread Safe (With Example)
Posted by
Brad Wood
Dec 04, 2009 00:58:00 UTC
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.
[code]
<!-----------------------------------------------------------------------
********************************************************************************
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 :
This is the base component used to provide Application.cfc support.
----------------------------------------------------------------------->
<cfcomponent name="coldbox" hint="This is the base component used to provide Application.cfc support" output="false">
<!------------------------------------------- CONSTRUCTOR ------------------------------------------->
<!--- Constructor --->
<cfparam name="variables.COLDBOX_CONFIG_FILE" default="" type="string">
<cfparam name="variables.COLDBOX_APP_ROOT_PATH" default="#getDirectoryFromPath(getbaseTemplatePath())#" type="string">
<cfscript>
instance = structnew();
//Set the timeout
setLockTimeout(30);
//Set the app hash
setAppHash(hash(getBaseTemplatePath()));
//Set the COLDBOX CONFIG FILE
setCOLDBOX_CONFIG_FILE(COLDBOX_CONFIG_FILE);
//Set the Root
setCOLDBOX_APP_ROOT_PATH(COLDBOX_APP_ROOT_PATH);
</cfscript>
<!------------------------------------------- PUBLIC ------------------------------------------->
<!--- Init --->
<cffunction name="init" access="public" returntype="coldbox" hint="Used when not using inheritance" output="false" >
<cfargument name="COLDBOX_CONFIG_FILE" required="true" type="string" hint="The coldbox config file from the application.cfc">
<cfargument name="COLDBOX_APP_ROOT_PATH" required="true" type="string" hint="The coldbox app root path from the application.cfc">
<cfscript>
/* Set vars for two main locations */
setCOLDBOX_CONFIG_FILE(arguments.COLDBOX_CONFIG_FILE);
setCOLDBOX_APP_ROOT_PATH(arguments.COLDBOX_APP_ROOT_PATH);
return this;
</cfscript>
</cffunction>
<!--- Load ColdBox --->
<cffunction name="loadColdbox" access="public" returntype="void" hint="Load the framework" output="false" >
<!--- Clean up If Necessary --->
<cfif structkeyExists(application,"cbController")>
<cfset structDelete(application,"cbController")>
</cfif>
<!--- Create Brand New Controller --->
<cfset application.cbController = CreateObject("component","coldbox.system.controller").init(COLDBOX_APP_ROOT_PATH)>
<!--- Setup the Framework And Application --->
<cfset application.cbController.getLoaderService().setupCalls(COLDBOX_CONFIG_FILE)>
</cffunction>
<!--- Reload Checks --->
<cffunction name="reloadChecks" access="public" returntype="void" hint="Reload checks and reload settings." output="false" >
<cfset var ExceptionService = "">
<cfset var ExceptionBean = "">
<!--- Initialize the Controller If Needed--->
<cfif not structkeyExists(application,"cbController") or not application.cbController.getColdboxInitiated() or isfwReinit()>
<cflock type="exclusive" name="#getAppHash()#" timeout="#getLockTimeout()#" throwontimeout="true">
<cfif not structkeyExists(application,"cbController") or not application.cbController.getColdboxInitiated() or isfwReinit()>
<cfset loadColdBox()>
<cfset application.cbController.getPlugin("logger").logEntry("information", "Application #application.cbController.getSetting('AppName')# Reinitialized", "")>
<cfset application.cbController.getPlugin("MessageBox").setMessage("info","Application #application.cbController.getSetting('AppName')# Reinitialized on server #server.hostName#")>
</cfif>
</cflock>
<cfelse>
<cftry>
<!--- AutoReload Tests --->
<cfif application.cbController.getSetting("ConfigAutoReload")>
<cflock type="exclusive" name="#getAppHash()#" timeout="#getLockTimeout()#" throwontimeout="true">
<cfset application.cbController.setAppStartHandlerFired(false)>
<cfset application.cbController.getLoaderService().setupCalls(COLDBOX_CONFIG_FILE)>
</cflock>
<cfelse>
<!--- Handler's Index Auto Reload --->
<cfif application.cbController.getSetting("HandlersIndexAutoReload")>
<cflock type="exclusive" name="#getAppHash()#" timeout="#getLockTimeout()#" throwontimeout="true">
<cfset application.cbController.getHandlerService().registerHandlers()>
</cflock>
</cfif>
<!--- IOC Framework Reload --->
<cfif application.cbController.getSetting("IOCFrameworkReload")>
<cflock type="exclusive" name="#getAppHash()#" timeout="#getLockTimeout()#" throwontimeout="true">
<cfset application.cbController.getPlugin("ioc").configure()>
</cflock>
</cfif>
</cfif>
<!--- Trap Framework Errors --->
<cfcatch type="any">
<cfset ExceptionService = application.cbController.getExceptionService()>
<cfset ExceptionBean = ExceptionService.ExceptionHandler(cfcatch,"framework","Framework Initialization/Configuration Exception")>
<cfoutput>#ExceptionService.renderBugReport(ExceptionBean)#</cfoutput>
<cfabort>
</cfcatch>
</cftry>
</cfif>
</cffunction>
<!--- Process A ColdBox Request --->
<cffunction name="processColdBoxRequest" access="public" returntype="void" hint="Process a Coldbox Request" output="true" >
<cfset var cbController = 0>
<cfset var Event = 0>
<cfset var ExceptionService = 0>
<cfset var ExceptionBean = 0>
<cfset var renderedContent = "">
<cfset var eventCacheEntry = 0>
<cfset var interceptorData = structnew()>
<!--- Start Application Requests --->
<cflock type="readonly" name="#getAppHash()#" timeout="#getLockTimeout()#" throwontimeout="true">
<cftry>
<!--- set request time --->
<cfset request.fwExecTime = getTickCount()>
<!--- Local Reference --->
<cfset cbController = application.cbController>
<!--- Create Request Context & Capture Request --->
<cfset Event = cbController.getRequestService().requestCapture()>
<!--- Debugging Monitors & Commands Check --->
<cfif cbController.getDebuggerService().getDebugMode()>
<!--- Commands Check --->
<cfset coldboxCommands(cbController,event)>
<!--- Which panel to render --->
<cfif event.getValue("debugPanel","") eq "cache">
<cfoutput>#cbController.getDebuggerService().renderCachePanel()#</cfoutput>
<cfabort>
<cfelseif event.getValue("debugPanel","") eq "cacheviewer">
<cfoutput>#cbController.getDebuggerService().renderCacheDumper()#</cfoutput>
<cfabort>
<cfelseif event.getValue("debugPanel","") eq "profiler">
<cfoutput>#cbController.getDebuggerService().renderProfiler()#</cfoutput>
<cfabort>
</cfif>
</cfif>
<!--- Application Start Handler --->
<cfif cbController.getSetting("ApplicationStartHandler") neq "" and (not cbController.getAppStartHandlerFired())>
<cfset cbController.runEvent(cbController.getSetting("ApplicationStartHandler"),true)>
<cfset cbController.setAppStartHandlerFired(true)>
</cfif>
<!--- Execute preProcess Interception --->
<cfset cbController.getInterceptorService().processState("preProcess")>
<!--- IF Found in config, run onRequestStart Handler --->
<cfif cbController.getSetting("RequestStartHandler") neq "">
<cfset cbController.runEvent(cbController.getSetting("RequestStartHandler"),true)>
</cfif>
<!--- Before Any Execution, do we have cached content to deliver --->
<cfif Event.isEventCacheable() and cbController.getColdboxOCM().lookup(Event.getEventCacheableEntry())>
<cfset renderedContent = cbController.getColdboxOCM().get(Event.getEventCacheableEntry())>
<cfoutput>#renderedContent#</cfoutput>
<cfelse>
<!--- Run Default/Set Event --->
<cfset cbController.runEvent(default=true)>
<!--- No Render Test --->
<cfif not event.isNoRender()>
<!--- Check for Marshalling and data render --->
<cfif isStruct(event.getRenderData()) and not structisEmpty(event.getRenderData())>
<cfset renderedContent = cbController.getPlugin("Utilities").marshallData(argumentCollection=event.getRenderData())>
<cfelse>
<!--- Render Layout/View pair via set variable to eliminate whitespace--->
<cfset renderedContent = cbController.getPlugin("renderer").renderLayout()>
</cfif>
<!--- PreRender Data:--->
<cfset interceptorData.renderedContent = renderedContent>
<!--- Execute preRender Interception --->
<cfset cbController.getInterceptorService().processState("preRender",interceptorData)>
<!--- Check if caching the content --->
<cfif event.isEventCacheable()>
<cfset eventCacheEntry = Event.getEventCacheableEntry()>
<!--- Cache the content of the event --->
<cfset cbController.getColdboxOCM().set(eventCacheEntry.cacheKey,
renderedContent,
eventCacheEntry.timeout,
eventCacheEntry.lastAccessTimeout)>
</cfif>
<!--- Render Content Type if using Render Data --->
<cfif isStruct(event.getRenderData()) and not structisEmpty(event.getRenderData())>
<!--- Render the Data Content Type --->
<cfcontent type="#event.getRenderData().contentType#" reset="true">
<!--- Remove panels --->
<cfsetting showdebugoutput="false">
<cfset event.showDebugPanel(false)>
</cfif>
<!--- Render the Content --->
<cfoutput>#renderedContent#</cfoutput>
<!--- Execute postRender Interception --->
<cfset cbController.getInterceptorService().processState("postRender")>
</cfif>
<!--- If Found in config, run onRequestEnd Handler --->
<cfif cbController.getSetting("RequestEndHandler") neq "">
<cfset cbController.runEvent(cbController.getSetting("RequestEndHandler"),true)>
</cfif>
<!--- Execute postProcess Interception --->
<cfset cbController.getInterceptorService().processState("postProcess")>
<!--- End else if not cached event --->
</cfif>
<!--- Trap Application Errors --->
<cfcatch type="any">
<!--- Get Exception Service --->
<cfset ExceptionService = cbController.getExceptionService()>
<!--- Intercept The Exception --->
<cfset interceptorData = structnew()>
<cfset interceptorData.exception = cfcatch>
<cfset cbController.getInterceptorService().processState("onException",interceptorData)>
<!--- Handle The Exception --->
<cfset ExceptionBean = ExceptionService.ExceptionHandler(cfcatch,"application","Application Execution Exception")>
<!--- Render The Exception --->
<cfoutput>#ExceptionService.renderBugReport(ExceptionBean)#</cfoutput>
</cfcatch>
</cftry>
<!--- DebugMode Routines --->
<cfif cbController.getDebuggerService().getDebugMode()>
<!--- Request Profilers --->
<cfif cbController.getDebuggerService().getDebuggerConfigBean().getPersistentRequestProfiler() and
structKeyExists(request,"debugTimers")>
<cfset cbController.getDebuggerService().pushProfiler(request.DebugTimers)>
</cfif>
<!--- Render DebugPanel --->
<cfif Event.getdebugpanelFlag()>
<!--- Time the request --->
<cfset request.fwExecTime = GetTickCount() - request.fwExecTime>
<!--- Render Debug Log --->
<cfoutput>#cbController.getDebuggerService().renderDebugLog()#</cfoutput>
</cfif>
</cfif>
</cflock>
</cffunction>
<!--- Session Start --->
<cffunction name="onSessionStart" returnType="void" output="false" hint="An onSessionStart method to use or call from your Application.cfc">
<cfset var cbController = "">
<cflock type="readonly" name="#getAppHash()#" timeout="#getLockTimeout()#" throwontimeout="true">
<cfscript>
//Setup the local controller
cbController = application.cbController;
//Execute Session Start interceptors
cbController.getInterceptorService().processState("sessionStart",session);
//Execute Session Start Handler
if ( cbController.getSetting("SessionStartHandler") neq "" ){
cbController.runEvent(cbController.getSetting("SessionStartHandler"),true);
}
</cfscript>
</cflock>
</cffunction>
<!--- Session End --->
<cffunction name="onSessionEnd" returnType="void" output="false" hint="An onSessionEnd method to use or call from your Application.cfc">
<!--- ************************************************************* --->
<cfargument name="sessionScope" type="struct" required="true">
<cfargument name="appScope" type="struct" required="false">
<!--- ************************************************************* --->
<cfset var cbController = "">
<cfset var event = "">
<cflock type="readonly" name="#getAppHash()#" timeout="#getLockTimeout()#" throwontimeout="true">
<cfscript>
//Check for cb Controller
if ( structKeyExists(arguments.appScope,"cbController") ){
//setup the controller
cbController = arguments.appScope.cbController;
event = cbController.getRequestService().getContext();
//Execute Session End interceptors
cbController.getInterceptorService().processState("sessionEnd",arguments.sessionScope);
//Execute Session End Handler
if ( cbController.getSetting("SessionEndHandler") neq "" ){
//Place session reference on event object
event.setValue("sessionReference", arguments.sessionScope);
//Place app reference on event object
event.setValue("applicationReference", arguments.appScope);
//Execute the Handler
cbController.runEvent(event=cbController.getSetting("SessionEndHandler"),
prepostExempt=true,
default=true);
}
}
</cfscript>
</cflock>
</cffunction>
<!--- setter COLDBOX CONFIG FILE --->
<cffunction name="setCOLDBOX_CONFIG_FILE" access="public" output="false" returntype="void" hint="Set COLDBOX_CONFIG_FILE">
<cfargument name="COLDBOX_CONFIG_FILE" type="string" required="true"/>
<cfset variables.COLDBOX_CONFIG_FILE = arguments.COLDBOX_CONFIG_FILE/>
</cffunction>
<!--- setter COLDBOX_APP_ROOT_PATH --->
<cffunction name="setCOLDBOX_APP_ROOT_PATH" access="public" output="false" returntype="void" hint="Set COLDBOX_APP_ROOT_PATH">
<cfargument name="COLDBOX_APP_ROOT_PATH" type="string" required="true"/>
<cfset variables.COLDBOX_APP_ROOT_PATH = arguments.COLDBOX_APP_ROOT_PATH/>
</cffunction>
<!--- FW needs reinit --->
<cffunction name="isfwReinit" access="public" returntype="boolean" hint="Verify if we need to reboot the framework" output="false" >
<cfset var reinitPass = "">
<cfset var incomingPass = "">
<!--- CF Parm Structures just in case. --->
<cfparam name="FORM" default="#StructNew()#">
<cfparam name="URL" default="#StructNew()#">
<cfscript>
/* Check if we have a reinit password at hand. */
if ( application.cbController.settingExists("ReinitPassword") ){
reinitPass = application.cbController.getSetting("ReinitPassword");
}
/* Verify the reinit key is passed */
if ( structKeyExists(url,"fwreinit") or structKeyExists(form,"fwreinit") ){
/* pass Checks */
if ( reinitPass eq "" ){
return true;
}
else{
/* Get the incoming pass from form or url */
if( structKeyExists(form,"fwreinit") ){
incomingPass = form.fwreinit;
}
else{
incomingPass = url.fwreinit;
}
/* Compare the passwords */
if( Compare(reinitPass, incomingPass) eq 0 ){
return true;
}
else{
return false;
}
}//end if reinitpass neq ""
}//else if reinit found.
else{
return false;
}
</cfscript>
</cffunction>
<!------------------------------------------- PRIVATE ------------------------------------------->
<!--- coldboxCommands --->
<cffunction name="coldboxCommands" output="false" access="private" returntype="void" hint="Execute some coldbox commands">
<cfargument name="cbController" type="any" required="true" default="" hint="The cb Controller"/>
<cfargument name="event" type="any" required="true" hint="An event context object"/>
<cfset var command = event.getTrimValue("cbox_command","")>
<cfscript>
/* Verify it */
if( len(command) eq 0 ){ return; }
/* Commands */
switch(command){
case "expirecache" : {cbController.getColdboxOCM().expireAll();break;}
case "delcacheentry" : {cbController.getColdboxOCM().clearKey(event.getValue('cbox_cacheentry',""));break;}
case "clearallevents" : {cbController.getColdboxOCM().clearAllEvents();break;}
case "clearallviews" : {cbController.getColdboxOCM().clearAllViews();break;}
default: break;
}
</cfscript>
<!--- Relocate --->
<cfif event.getValue("debugPanel","") eq "">
<cflocation url="index.cfm" addtoken="false">
<cfelse>
<cflocation url="index.cfm?debugpanel=#event.getValue('debugPanel','')#" addtoken="false">
</cfif>
</cffunction>
<!--- Getter setter lock timeout --->
<cffunction name="getLockTimeout" access="private" output="false" returntype="numeric" hint="Get LockTimeout">
<cfreturn instance.LockTimeout/>
</cffunction>
<cffunction name="setLockTimeout" access="private" output="false" returntype="void" hint="Set LockTimeout">
<cfargument name="LockTimeout" type="numeric" required="true"/>
<cfset instance.LockTimeout = arguments.LockTimeout/>
</cffunction>
<!--- AppHash --->
<cffunction name="getAppHash" access="private" output="false" returntype="string" hint="Get AppHash">
<cfreturn instance.AppHash/>
</cffunction>
<cffunction name="setAppHash" access="private" output="false" returntype="void" hint="Set AppHash">
<cfargument name="AppHash" type="string" required="true"/>
<cfset instance.AppHash = arguments.AppHash/>
</cffunction>
</cfcomponent>
[/code]
Comments are currently closed
marc esher
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.
Jason Delmore
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
Brad Wood
That's awesome, Jason! Thanks for the update. I really like caching idea.