Aspx, global instance of class, possible bug in code structure

Question

I am tracking down a bug in some old aspx code. The problem is that one some very rare occations (1/10.000 pageviews or so) two users are mixed up, ie. user A sees user B data.

Here is how the code is structured: We have a user class which is defined in a module like this:

Public Module MyGlobals
    Public myUser As CMyUser
End Module

On the loginpage, we validate the username/password and if valid then the coorosponding userid is loaded from db, and we do:

FormsAuthentication.SetAuthCookie(userid, False)

Then we redirect to the secure area. In the secure areas MasterPage, on event Page_Init, we then have:

If Context.User.Identity.IsAuthenticated then
    ' Initialize the user class (user data is loaded)
    MyGlobals.myUser = New CMyUser(Context.User.Identity.Name)
Else
    ' Redirect to loginpage
End If

Hereafter, is it safe to access the

MyGlobals.myUser

instance from every page which has the secure masterpage as masterpage, or could there be issues with this structure?

Dennadennard

Wouldn't the Globals object fall out of scope at the end of every page request? – Autoicous

Answer 1

A VB.Net Module is like a static class with a private constructor and only static fields in C#.

That means, all variables declared in a module are shared across all threads. Hence every request(User) that's using this module will overwrite the old value.

I would strongly recommend to use Session to store user-sensitive data. But i'm not sure why you want to store the Username because it's already stored when using FormsAuthentication(as you've shown yourself above).

If you really need this wrapper, you could easily achieve it even in a static context via HttpContext.Current.Session:

Module MyGlobals
    Public Property myUser As CMyUser
        Get
            If HttpContext.Current.Session("CurrentUser") Is Nothing Then
                Return Nothing
            Else
                Return DirectCast(HttpContext.Current.Session("CurrentUser"), CMyUser)
            End If
        End Get
        Set(ByVal value As CMyUser)
            HttpContext.Current.Session("CurrentUser") = value
        End Set
    End Property
End Module

Struma

Thanks for explaining this, could very well be the source of our problems... The CMyUser class loads user data from db (like name, mail, phone, prefered language etc). We put it in a global module so that we only need to load it one place (masterpage), and then access it like MyGlobals.myUser.name_first or MyGlobals.myUser.mail. – Dennadennard

If you really need this, you could easily achieve it by making myUser a property and using HtppContext.Current.Session to store it. Wait, i will edit my answer. – Struma

Thanks a lot, seems to work. Before it also worked except from maybe one pageload out of 10.000 or so, probably two users loading a page in same small timeinterval. Would you say that the problem we had could be explained by having declared the myUser as a public in a module? – Dennadennard

What you've done is nothing that increases the performance, because the masterPage's Page_Init will anyway be called on every postback on any page of every user. So i don't understand the benefit. But yes, it might be the reason for your problems, because the page needs more or less time to pass through the Page-Life-Cycle, hence when User#2 will overwrite this value meanwhile and User#1 retrieves the value afterwards, then you have your big-bang.(btw, please remember accept answers if they are mostly correct) – Struma