Tales from the trenches: ASP.Net ViewState bug


This post is part of a series of actual problems or bad practices I encountered working on existing C# code bases.

The names of all identifiers, the exact functionalities of the programs and the code samples have all been altered for confidentiality reasons.

While looking into a slow ASP.Net page I noticed that the page was making a lot more service calls than it should have. Visually I could see superfluous calls that should have been merged together.

I profiled the page with the Performance Tools in Visual Studio and also put breakpoints on all service calls and ran it in Debug mode.

One thing that stood out was one property that was supposed to be using ViewState to prevent calls to the service layer but actually wasn’t.

It went something like this :

public int Somevalue 
{
    get 
    {
        if (ViewState["SomeValue"] == null)
        {
            var foo =
            ValueService.GetSomeValue(Convert.ToInt32
                (Request["SomeValue"]));

            ViewState["SomeValue"] = foo;
        }
        
        return (int)ViewState["SomeValue"];
    }
    set 
    {
        ViewState["SomeValue"] = value;
    }
}

// ...

protected override void OnInit(EventArgs e)
{
    int x = SomeValue;	// contrived example
}

I noticed that ValueService.GetSomeValue was getting called on PostBacks. Looking at where it was used, I saw it was being called in the OnInit method. It just so happens that during the OnInit phase of the ASP.Net page life cycle, the ViewState hasn’t be loaded with it’s values. This effectively means that SomeValue will always be null in the OnInit phase and it will have to make a service call every time.

The solution in this case was simply to move the call to a later stage of the page life cycle, in this case the PageLoad event.

The take away from this is to be mindful of the ASP.Net page life cycle events. Also putting breakpoints on service calls while loading the page and submitting the form is a quick way to see if something is amiss with your service calls.

Tales from the trenches: ASP.Net static variable bug


This post is part of a series of actual problems or bad practices I encountered working on existing C# code bases.

The names of all identifiers, the exact functionalities of the programs and the code samples have all been altered for confidentiality reasons.

While working on an ASP.Net control bug, I noticed this more important problem which hadn’t been reported.

The reported bug was about a control that would sometimes not repopulate itself. It was a pretty simple case.

if (ConditionA && lastValue != currentValue)
{
    lastValue == currentValue;
    // more code
}

What happened is that when ConditionA failed, lastValue wasn’t updated. The fix was simply to move the lastValue assignment statement outside of the conditional.

The bigger problem I noticed was when I decided to look at how lastValue was implemented.

It turned out it was implemented as a static member variable. This being an ASP.Net page, a static variable is shared between all instances of the class in the AppDomain.

What this effectively means is that every user of the site shares this same variable.

Suppose user A fills lastValue with “test”, then user B fills currentValue with “test”, the server-side code detects that user B has already used this value as his value (since it’s currently in lastvalue), even if he hasn’t entered anything before. This could lead to some inconsistent behaviour.

The lesson here is two-fold:

1- Do not to cache information with a static variable in an ASP.Net site unless that information is not subject to change from user to user.

2- When working on existing code, make sure you understand correctly how it is implemented. In this case I could have glossed over the implementation of lastValue, which wasn’t declared near the method I was working on and missed this important bug.