views:

112

answers:

2

I am looking at a design pattern which has come up in quite a few of my firm's projects. It has historically functioned correctly, however I have heard some other developers argue that there is a possibility of session corruption using this pattern. I'm looking for insight from other .NET developers here on Stack Overflow.

Basically, there's a class -- usually either static or a Singleton pattern, depending largely on the developer who wrote it -- stored in App_Code.

This class encapsulates access to the current session via properties. All of these properties take the form of:

public static class SessionHelper
{
    public static string SessionValue
    {
        get
        {
            object o = HttpContext.Current.Session["sessionValueName"];
            if (o == null)
            {
                // Replace the following with code to store & retrieve
                // a default value of the appropriate datatype.
                o = string.Empty;
                HttpContext.Current.Session["sessionValueName"] = o;
            }

            return o.ToString(); // or cast, ensure cast is valid & return.
        }
        set
        {
            HttpContext.Current.Session["sessionValueName"] = value;
        }
    }

    // Other properties, strongly-typed, as above.
}

(These are not static when the class is a Singleton.)

I have seen issues with static data on web sites in the past, largely because static data was used to maintain per-session state. (I have, for example, seen statically-declared "per-user" members of code-behinds. Not my code; my team was the clean-up crew for the company that wrote that mess.)

However, because this is just a static entry to HttpContext.Current.Session, it seems like it should be safe, as it is not fundamentally any different than the Page class encapsulating this in the Session property. As I said, no other site on which my company has worked that used this pattern saw it ever have any issues -- and that includes some pretty large and highly active userbases. But I want to just get a fresh perspective.

Are there potential multi-user issues, race conditions, or other failings/flaws which specifically could cause session corruption in the above pattern?

+2  A: 

I'm using a very similar approach as the one you presented and have not found any problems so far.

Maybe one thing to be concerned about is the hard coded session keys: you can never be sure that the same key isn't used for a different purpose in another location.

That's why I'm relying on only one key, used to store "my session" in the ASP.NET session. All session data is then implemented as normal properties of the "my session" object.


Here's how my approach looks like:

public class MySession
{
    // private constructor
    private MySession() {}

    // Gets the current session.
    public static MySession Current
    {
      get
      {
        MySession session =
          (MySession)HttpContext.Current.Session["__MySession__"];
        if (session == null)
        {
          session = new MySession();
          HttpContext.Current.Session["__MySession__"] = session;
        }
        return session;
      }
    }

    // **** add your session properties here, e.g like this:
    public string Property1 { get; set; }
    public DateTime MyDate { get; set; }
    public int LoginId { get; set; }
}

This class stores one instance of itself in the ASP.NET session and allows you to access your session properties in a type-safe way from any class, e.g like this:

int loginId = MySession.Current.LoginId;

string property1 = MySession.Current.Property1;
MySession.Current.Property1 = newValue;

DateTime myDate = MySession.Current.MyDate;
MySession.Current.MyDate = DateTime.Now;

This approach has several advantages:

  • it saves you from a lot of type-casting
  • you don't have to use hard-coded session keys throughout your application (e.g. Session["loginId"]
  • you can document your session items by adding XML doc comments on the properties of MySession
M4N
+1, I *think* we're generally on the same page; I don't think I was clear that this is a separate class, and the hard-coded keys are only used in this class, as all session access goes through this class. I will update my question text appropriately. However, I kind of like the single-object-in-session idea, which is novel. :)
John Rudy
Hmmmm... I don't really like this approach. It means you are you are getting/setting EVERYTHING each time you access a single Session item. This could be an expensive operation, depending on your Session provider.
Bryan
@Bryan: that's a good point! But generally we try to keep the session state as small as possible, and the difference between getting one object containing 20 values (as in my approach) or getting 5 or 10 values separately will probably not be big.
M4N
+1  A: 

This is safe. The fact you are accessing it through some static property or static class is completely irrelevant. That is a language abstraction. The Session provider is what is responsible for mapping the requests to their correct session... so unless there is a bug in this, you are A-OK.

Bryan