Is this code thread-safe? How can I make it thread-safe?

Question

I have a WCF service with a security class for getting some of the attributes of the calling user. However I'm quite bad when it comes to thread safety - to this point, I haven't needed to do much with it, and only have a rudimentary theoretical understanding of the problems of multi-threading.

Given the following function:

public class SecurityService
{
    public static Guid GetCurrentUserID()
    {
        if (Thread.CurrentPrincipal is MyCustomPrincipal)
        {
            MyCustomIdentity identity = null;
            MyCustomPrincipal principal = (MyCustomPrincipal)Thread.CurrentPrincipal;
            if (principal != null)
            {
                identity = (MyCustomIdentity)principal.Identity;
            }

            if (identity != null)
            {
                return identity.UUID;
            }
        }
        return Guid.Empty;
    }
}

Is there any chance that something could go wrong in there if the method is being called at the same time from 2 different threads? In my nightmares I see terrible consequences if these methods go wrong, like someone accidentally getting someone else's data or suddenly becoming a system administrator. A colleague (who also he was not an expert, but he's better than me) thought it would probably be okay because there's not really any shared resources that are being accessed there.

Or this one, which will access the database - could this go awry?

    public static User GetCurrentUser()
    {
        var uuid = GetCurrentUserID();
        if (uuid != null)
        {
            var rUser = new UserRepository();
            return rUser.GetByID(uuid);
        }
        return null;
    }

There's a lot of discussion about the principals of threading, but I tend to fall down and get confused when it comes to actually applying it, and knowing when to apply it. Any help appreciated.

I can explain more about the context/purpose of these functions if it's not clear.

EDIT: The rUser.GetByID() function basically calls through to a repository that looks up the database using NHibernate. So I guess the database here is a "shared resource", but not really one that gets locked or modified for this operation... in which case I guess it's okay...?

Answer 1

From what I see, the first example only accesses thread-local storage and stack-based variables, while the second one only accesses stack-based variables.

Both should be thread-safe.

I can't tell if GetByID is thread safe or not. Look to see if it accesses any shared/static resources. If it does, it's not thread-safe without some additional code to protect those resources.

Answer 2

The code that you have above doesn't contain any code that changes global state, therefore you can be fairly sure that it won't be a problem being called by multiple simlutaneous threads. Security principal information is tied to each thread, so no problem there either.