What's Wrong With This Code (#12)

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?

Print | posted @ Tuesday, February 13, 2007 4:57 AM

Comments on this entry:

Gravatar # re: What's Wrong With This Code (#12)
by Name: required at 2/13/2007 6:00 AM

startTime should not be static.
  
Gravatar # re: What's Wrong With This Code (#12)
by Haacked at 2/13/2007 7:02 AM

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.
  
Gravatar # re: What's Wrong With This Code (#12)
by jayson knight at 2/13/2007 10:07 AM

Init is called each time this httpModule is called...using something like

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

should provide the correct value for subsequent requests.
  
Gravatar # re: What's Wrong With This Code (#12)
by KraGiE at 2/13/2007 11:29 AM

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.
  
Gravatar # re: What's Wrong With This Code (#12)
by scott at 2/13/2007 1:18 PM

You guys nailed that one.
  
Gravatar # re: What's Wrong With This Code (#12)
by scott at 2/13/2007 1:21 PM

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 have been closed on this topic.
Scott Allen
Posts - 869
Comments - 4493
Stories - 14