What's Wrong With This Code (#12)

Tuesday, February 13, 2007

Joe Developer has been fired. Sacked. Terminated. Dismissed. Booted. "Dis-employed", if you will.

Before Joe left, he started to write one last piece of code. He was asked to write an HTTP Module in ASP.NET that will log some information, and will include the date and time when the application last started running. He learned his lesson in last week's misadventure (too little, too late), and made his DateTime field static, like so:

using System;
using System.Web;

public class LoggingModule : IHttpModule
{
    public void Init(HttpApplication context)
    {
        startTime = DateTime.Now;
    }

    public void Dispose()
    {
    }

    static DateTime startTime;
}

Joe's replacement, Estelle Hertz, has to finish the module Joe started. Estelle thinks Joe was off to a pretty good start and adds some event handlers, some logging code, and checks in her changes.

The testers are finding that the "start time" recorded by her module is drifting, but the event logs show the application is definitely not restarting.

Did Joe leave a bug behind?

Comments
Name: required Tuesday, February 13, 2007
startTime should not be static.
Haacked Tuesday, February 13, 2007
He sure did, the oaf! ASP.NET can instantiate more than one HttpApplication instance in order to handle requests.

So the start time will show the time of the last HttpApplication instance initialized, not the start time of the ASP.NET application itself.
jayson knight Tuesday, February 13, 2007
Init is called each time this httpModule is called...using something like

startTime = (DateTime)Application["AppStartTime"];

should provide the correct value for subsequent requests.
KraGiE Tuesday, February 13, 2007
the static declaration of startTime is always set on Init. Meaning every time the module is instantiated, it'll alter the static member.

Should have put this in the function
if (startTime == DateTime.MinValue)
lock (padlock)
startTime = DateTime.Now;

and declare a static object instantiated as a plain object.
scott Tuesday, February 13, 2007
You guys nailed that one.
scott Tuesday, February 13, 2007
Just to be clear, if Joe wrote:

static DateTime startTime = DateTime.Now;

he'd be ok. The problem is, as Phil pointed out, is that ASP.NET will instantiate multiple modules during the life of the web app, and each one has the Init method called. The startTime will keep changing each time one is created.

By initializing the static like the above, it's only assigned a value with the first module that is created.
Comments are now closed.
Follow Me On Twitter
RSS Subscribe
Contact
Search Archives About
OdeToCode by K. Scott Allen
K.Scott Allen