How did I get this NullReferenceException error here right after the constructor?

Question

I've had an asp.net website running live on our intranet for a couple of weeks now. I just got an email from my application_error emailer method with an unhandled exception.

Here it is (ive cleaned up some of the paths to make it better displayed)

Exception : Object reference not set to an instance of an object. Stack Trace : at System.Collections.Generic.Dictionary2.Insert(TKey key, TValue value, Boolean add) at System.Collections.Generic.Dictionary2.Add(TKey key, TValue value) at TimesheetDomain.DataMappers.StaffMemberData.ReadStaff(SqlDataReader reader) in TimesheetDomain\DataMappers\StaffMemberData.cs:line 362

at TimesheetDomain.DataMappers.StaffMemberData.GetStaffMember(String name) in TimesheetDomain\DataMappers\StaffMemberData.cs:line 401

at TimesheetDomain.ServiceLayer.TimesheetManager.GetUserFromName(String name) in TimesheetDomain\ServiceLayer\TimesheetManager.cs:line 199

at UserVerification.GetCurrentUser() in \App_Code\UserVerification.cs:line 29 at WebTimesheets.OnInit(EventArgs e) in \WebTimesheets\WebTimesheets.master.cs:line 159

at System.Web.UI.Control.InitRecursive(Control namingContainer) at System.Web.UI.Control.InitRecursive(Control namingContainer) at System.Web.UI.Page.ProcessRequestMain(Boolean includeStagesBeforeAsyncPoint, Boolean includeStagesAfterAsyncPoint)

Basically it looks like its erroring at my ReadStaff method which reads a data reader to build staff member objects. Here is the bit of code:

            while (reader != null && reader.Read())
        {
            StaffMember newMember = null;
            string firstName = reader["FirstName"].ToString();
            string lastName = reader["LastName"].ToString();
            int staffID = (int)reader["StaffID"];
            int employSection = (int)reader["EmploySection"];
            StaffType employType = (StaffType)employSection;
            string emailAddress = reader["EmailInt"].ToString();
            int employCode = (int)reader["ibbwid"];

            //check if they are an admin staff member 
            if (IsAdminStaff(employType))
            {
                newMember = new AdminOfficer(firstName, lastName, employType, staffID, emailAddress, employCode);
            }
            else
            {
                //check if they are a supervisor
                if (IsASupervisor(staffID))
                    newMember = new Supervisor(firstName, lastName, employType, staffID, emailAddress, employCode);
                else
                    newMember = new StaffMember(firstName, lastName, employType, staffID, emailAddress, employCode);
            }

            //add to identity map
            if (!_staffMembers.ContainsKey(staffID))
                _staffMembers.Add(staffID, newMember); //****THIS IS LINE 362*****
            else
                _staffMembers[staffID] = newMember;

(Line 362 is 3rd last line) I'm using an identity map (just read fowlers book on patterns and thought it was a good idea - may have done it wrong, happy for comments) but thats not overly relevant as later on I use the newMember object elsewhere so if I remove that block the nullreference will happen then.

I am struggling to see how on earth newMember is null in the 3rd last line there (which is the line that errored)

Resharper/VS dont give me a warning that it could be null - because theres the 3 constructors which i choose from.

Can anyone suggest where I can look to try and fix this error? It's only happened once and that method has been called thousands of times since the site went live.

Thanks

[EDIT] As Requested, here's the IComparer for staff member

        /// <summary>
    /// Comparer for staff members - compares on name
    /// </summary>
    public class StaffMemberComparer : IComparer
    {
        public int Compare(object x, object y)
        {
            //check they are staff members
            if (x is StaffMember && y is StaffMember)
            {
                //do a simple string comparison on names
                StaffMember staffX = x as StaffMember;
                StaffMember staffY = y as StaffMember;

                return String.Compare(staffX.FirstName, staffY.FirstName);

            }
            throw new Exception("This is for comparing Staff Members");
        }
    }

and its used in the IComparable implementation

        /// <summary>
    /// IComparable implementaiton
    /// </summary>
    /// <param name="obj">object to compare to</param>
    /// <returns></returns>
    public int CompareTo(object obj)
    {
        StaffMemberComparer comparer = new StaffMemberComparer();
        return comparer.Compare(this, obj);
    }

Answer 1

What do those classes look like? Have you overridden .Equals or .GetHashCode()?

Answer 2

Have you overridden GetHashCode and Equals? What do they look like? Is the dictionary using a custom IEqualityComparer?

Answer 3

As others have said, the comparison could be causing the problem.
Is any of the criteria for comparison contain null value? specifically, the string properties?

i.e. If firstname or lastname or emailId is null and if it is used in comparison, things could fail when used inside the dictionary for comparison.

EDIT: How is Supervisor and StaffMember and AdminStaff class related?
In the code, you are casting both the instances to StaffMember. My guess is that it could be a problem if the Supervisor and StaffMember class are not related.

EDIT2: What is the scrope of the dictionary instance? Is it shared at application level/session level? Is it possible that multiple threads could try to read/write from it?

Answer 4

I cant see anything obvious. I'd run some SQL to check the database for any bad data. The problem may be a freak bug in a related input form. If the code has been run thousands of times without incident until now, i'd wrap some additional exception handling/reporting around the code block in question so you can at least get a staffId if/when it next happens.

You could burn a lot of time on something like this. The most expedient approach may be just to let it fail again under the above/controlled conditions..... assuming the level of disruption it causes is acceptable/manageable/minor.

I appreciate that wont satisfy the immediate need to know but it may be the best way to manage the problem especially with such a low failure rate.

Answer 5

It's only happened once and that method has been called thousands of times since the site went live.

After reading this, I can conclude that, its possible that .NET may have exhausted its memory and it could not create any more Dictionary key, it may not really be anywhere your fault. But yes we did get these kind of errors when we tried to store too much information in session/application variables thus increasing memory footprint of the Web Application. But we got such errors when our numbers went really high, like storing 10,000 items in Dictionary or List etc.

The pattern is good, but you must also realize that we use database to store information in relational format, if we start using memory to store similar things, then we are ignoring powerful database. Database can cache values for you as well.

It might sound silly but we have our windows server restart in every 24 hours, at midnight when there is no traffic. That did help us in getting rid of such errors. We restart our servers regularly at a fix schedule in order to get all the cache/logs cleared.

Answer 6

It is almost certainly a threading issue - see this question and its accepted answer.

Dictionary<>.Insert() will throw a NullReferenceException internally if the dictionary instance is modified from another thread during the insert operation.